fix: a few random crash fixes

[harrisony]

What type of PR is this?

• bug

What this PR does / why we need it:

Four fixes found while writing tests, one commit each.

validate — RequestError.Error() panicked when Err was nil. Guard it, fall back to status text.

middleware — bare .(string) on the request-ID key panics if chi's RequestID middleware didn't run; switched to comma-ok. Also the logger spy didn't override Write, so implicit-200 responses logged as status 0.

repo — NewGroupRepository never set attachments, so deleting a group with attachments panicked. Wired it through the constructor. Attachment deletes now use the transaction-bound client, avoiding the SQLite deadlock while keeping the database work in one transaction.

attachments — clearing the previous primary photo during attachment creation used the base client instead of the existing transaction. Run it through the transaction too.

Which issue(s) this PR fixes:

None.

Testing

• go test

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 12df63d1-5be3-4d1a-b44f-faa365470d89

📥 Commits

Reviewing files that changed from the base of the PR and between 8ffb7aa and e2e32a6.

📒 Files selected for processing (1)

• backend/internal/web/mid/logger.go

🚧 Files skipped from review as they are similar to previous changes (1)

• backend/internal/web/mid/logger.go

---

Summary by CodeRabbit

• Bug Fixes
  • Improved group deletion so associated attachments are removed within the same transaction.
  • Prevented attachment cleanup errors from interrupting group deletion.
  • Fixed error responses when no underlying error message is available.
  • Prevented request error handling and logging from failing when request IDs are missing or invalid.
  • Ensured attachment updates remain atomic during creation, including primary attachment changes.
• Logging
  • Improved response status tracking when data is written without an explicit status.

Walkthrough

Group deletion now cleans up attachments within the active transaction and rolls back failed initial queries. Primary attachment updates also use the transaction client, while error formatting and HTTP middleware tolerate nil errors and unexpected request values.

Changes

Repository transaction integrity

Layer / File(s)	Summary
Shared attachment repository wiring backend/internal/data/repo/repo_group.go, backend/internal/data/repo/repos_all.go	GroupRepository accepts and stores the shared AttachmentRepo, and repository initialization passes it through.
Transactional group deletion backend/internal/data/repo/repo_group.go	GroupDelete loads attachment edges in the transaction, rolls back failed initial queries, and deletes attachments through a transaction-bound repository while continuing after individual deletion errors.
Transactional primary attachment update backend/internal/data/repo/repo_item_attachments.go	Primary attachment creation clears other primary flags through the active transaction client.

Error and request logging robustness

Layer / File(s)	Summary
Safe error and response status handling backend/internal/sys/validate/errors.go, backend/internal/web/mid/errors.go, backend/internal/web/mid/logger.go	RequestError falls back to HTTP status text, request-ID extraction avoids panics, and response logging records implicit 200 OK writes.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Suggested reviewers: tankerkiller125

Poem

Attachments stay within the transaction,
Errors gain a clear fallback expression.
Trace IDs handle missing values with care,
Body writes record success in the air.
Safer middleware completes the progression.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name	Status	Explanation	Resolution
Title check	❓ Inconclusive	The title identifies crash fixes but uses the vague term "random" and omits the transaction-related changes.	Replace it with a specific summary, such as "fix: prevent crashes and correct repository transaction handling".

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description check	✅ Passed	The description identifies the bug category, explains the four fixes, states that no issues apply, and documents testing with go test.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches 💡 1

🛠️ Fix failing CI checks 💡

• Fix failing CI checks

✨ Simplify code

• Create PR with simplified code

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@backend/internal/data/repo/repo_group.go`:
- Around line 329-349: The group deletion flow around the attachment cleanup
loop and transaction must stop deleting blobs before the transaction or
continuing after cleanup errors. Make entity/group row deletion and durable
blob-cleanup intents part of the same transaction, then execute or retry blob
removals only after a successful commit, ensuring newly-created attachments and
transaction failures do not leave inconsistent cleanup state.

In `@backend/internal/sys/validate/errors.go`:
- Around line 63-66: Update RequestError.Error to return a non-empty fallback
when Err is nil and Status is 0, preferably by using http.StatusBadRequest
before calling http.StatusText. Add coverage for NewRequestError(nil, 0) that
verifies the returned error message is non-empty.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: bec2fbc9-9a82-4dbd-93a0-f44021aa491b

📥 Commits

Reviewing files that changed from the base of the PR and between 52eb6f3 and f54921c.

📒 Files selected for processing (5)

• backend/internal/data/repo/repo_group.go
• backend/internal/data/repo/repos_all.go
• backend/internal/sys/validate/errors.go
• backend/internal/web/mid/errors.go
• backend/internal/web/mid/logger.go

[harrisony]

I found some more thingies

@coderabbitai review

[coderabbitai]

@harrisony I’ll review the latest changes, with attention to error handling, transaction boundaries, and any security implications.

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.