Skip to content

chore: acknowledge unactionable errors explicitly#1773

Open
AmanGIT07 wants to merge 1 commit into
mainfrom
chore/explicit-error-ignores
Open

chore: acknowledge unactionable errors explicitly#1773
AmanGIT07 wants to merge 1 commit into
mainfrom
chore/explicit-error-ignores

Conversation

@AmanGIT07

Copy link
Copy Markdown
Contributor

Summary

Seven call sites ignore an error that is either expected or has no useful handling. Mark each with an explicit blank assignment or a nolint comment so the intent is visible.

Changes

  • internal/store/spicedb/relation_repository.go: // nolint on the three deferred New Relic nr.End() calls (a failure only loses one timing data point; keeps the defers as single statements)
  • pkg/server/server.go: _ = on the two response Encode calls (fails only when the client already disconnected)
  • internal/store/postgres/audit_record_repository.go: wrap the deferred tx.Rollback() safety net (returns ErrTxDone after a finished transaction by design)
  • test/e2e/testbench/testbench.go: _ = on the teardown self-interrupt signal

Technical Details

No behavior change; the compiled code is identical.

Test Plan

  • go test ./pkg/server/ ./internal/store/postgres/ passes
  • Build and type checking passes

SQL Safety (if your PR touches *_repository.go or goqu.*)

  • Values flow through ? placeholders, goqu.Ex{}, or goqu.Record{} — never fmt.Sprintf or + building a query that gets executed.
  • ToSQL() callers capture and forward params (query, params, err := stmt.ToSQL(); db.…Context(ctx, …, query, params...)). Never query, _, err := ….
  • No ? placeholders inside single-quoted SQL literals in goqu.L (use make_interval(hours => ?)-style functions instead).
  • Any //nolint:forbidigo or // #nosec G20x annotation has a one-line justification on the same line that a reviewer can verify.

Blank-assign or nolint-mark seven ignored error returns where nothing
useful can be done: New Relic segment ends, HTTP response encodes, the
deferred rollback safety net, and the e2e teardown signal.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Jul 17, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
frontier Ready Ready Preview, Comment Jul 17, 2026 12:31pm

@coderabbitai

coderabbitai Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 922e9b21-4c5f-447d-a7f5-25b4fc72a42b

📥 Commits

Reviewing files that changed from the base of the PR and between cfe2cf3 and 6b21e11.

📒 Files selected for processing (4)
  • internal/store/postgres/audit_record_repository.go
  • internal/store/spicedb/relation_repository.go
  • pkg/server/server.go
  • test/e2e/testbench/testbench.go

📝 Walkthrough

Summary by CodeRabbit

  • Chores
    • Improved static-analysis compliance across database, telemetry, server response, and test shutdown handling.
    • No user-facing functionality or API behavior changed.

Walkthrough

Lint suppressions and explicit blank-identifier assignments were added to deferred cleanup calls, JSON encoding calls, and process signaling. Runtime behavior and public interfaces remain unchanged.

Changes

Lint compliance cleanup

Layer / File(s) Summary
Deferred cleanup lint annotations
internal/store/postgres/audit_record_repository.go, internal/store/spicedb/relation_repository.go
Deferred rollback and New Relic segment termination calls now include // nolint annotations.
Explicitly discarded return values
pkg/server/server.go, test/e2e/testbench/testbench.go
JSON encoding and process-signal return values are explicitly assigned to _.

Estimated code review effort: 1 (Trivial) | ~5 minutes

Suggested reviewers: rohilsurana

🚥 Pre-merge checks | ✅ 2
✅ Passed checks (2 passed)
Check name Status Explanation
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.

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

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

@coveralls

Copy link
Copy Markdown

Coverage Report for CI Build 29580488960

Coverage remained the same at 46.11%

Details

  • Coverage remained the same as the base build.
  • Patch coverage: 5 uncovered changes across 3 files (1 of 6 lines covered, 16.67%).
  • No coverage regressions found.

Uncovered Changes

File Changed Covered %
internal/store/spicedb/relation_repository.go 3 0 0.0%
internal/store/postgres/audit_record_repository.go 1 0 0.0%
pkg/server/server.go 2 1 50.0%

Coverage Regressions

No coverage regressions found.


Coverage Stats

Coverage Status
Relevant Lines: 38321
Covered Lines: 17670
Line Coverage: 46.11%
Coverage Strength: 13.32 hits per line

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants