fix(middleware): guard typed-nil *fiber.Error matched by errors.As - #48
Conversation
errors.As matches a typed-nil *fiber.Error inside a joined chain (errors.Join((*fiber.Error)(nil), ...)) and leaves the target nil; reading .Code then panics with a nil-pointer dereference - inside the finalizer whose purpose is closing exactly this defect class. Confirmed by execution on the released line. Same fix shipped for the v2 maintenance line on fix/typed-nil-fiber-error-substitution-v2. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019J6AsXF9EbWtJ7CV7WtSdD
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe middleware now traverses wrapped and joined errors safely. Typed-nil ChangesHTTP error handling
Possibly related PRs
✨ Finishing Touches✨ Simplify code
Comment |
|
Pull requests to main can only come from:
Your source branch: Please change the base branch or create a PR from an allowed branch. |
🔍 PR Validation Summary✅ PR Mergeable — no blocking failures
|
🔒 Security Scan Results —
|
| Stage | Status | Blocking? |
|---|---|---|
| Filesystem Scan | ✅ Clean | — |
| Docker Image Scan | ➖ Skipped | — |
| Docker Hub Health Score | ➖ Skipped | — |
| Pre-release Version Check | ✅ Clean | — |
Trivy
Filesystem Scan
✅ No vulnerabilities or secrets found.
Pre-release Version Check
✅ No unstable version pins found.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@middleware/error_handling.go`:
- Around line 100-108: The error handling must skip typed-nil *fiber.Error
values and continue searching for a later valid Fiber error. Add a shared
extractor that traverses joined and wrapped error trees, returns only non-nil
*fiber.Error values, and reuse it in this normalization branch and
httpStatusCode; add regression coverage for a typed-nil Fiber error preceding a
valid one.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 810ed5b2-1c98-4ee6-8bdd-e796e598e488
📒 Files selected for processing (2)
middleware/error_handling.gomiddleware/error_handling_regression_test.go
📊 Unit Test Coverage Report:
|
| Metric | Value |
|---|---|
| Overall Coverage | 87.6% ✅ PASS |
| Threshold | 80% |
Coverage by Package
| Package | Coverage |
|---|---|
github.com/LerianStudio/lib-observability/v3/assert |
98.2% |
github.com/LerianStudio/lib-observability/v3/constants |
83.3% |
github.com/LerianStudio/lib-observability/v3/grpcmiddleware |
78.7% |
github.com/LerianStudio/lib-observability/v3/httpobs |
77.8% |
github.com/LerianStudio/lib-observability/v3/log |
90.7% |
github.com/LerianStudio/lib-observability/v3/messagingobs |
89.4% |
github.com/LerianStudio/lib-observability/v3/metrics |
91.4% |
github.com/LerianStudio/lib-observability/v3/middleware |
84.1% |
github.com/LerianStudio/lib-observability/v3/redaction |
95.8% |
github.com/LerianStudio/lib-observability/v3/redisobs |
63.1% |
github.com/LerianStudio/lib-observability/v3/runtime |
80.9% |
github.com/LerianStudio/lib-observability/v3/sqlobs |
64.2% |
github.com/LerianStudio/lib-observability/v3/telemetrycore |
84.2% |
github.com/LerianStudio/lib-observability/v3/tracing |
88.0% |
github.com/LerianStudio/lib-observability/v3/zap |
96.1% |
github.com/LerianStudio/lib-observability/v3 |
91.5% |
Generated by Go PR Analysis workflow
… joined chains X-Lerian-Ref: 0x1 Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@middleware/error_handling.go`:
- Around line 134-145: Update asFiberError to resolve the reported errorlint and
inamedparam violations while preserving its manual traversal and typed-nil
handling. Rename the parameter in the local As interface to satisfy inamedparam,
and add narrow linter suppressions for the intentional direct type assertions
and inspection in the fiber.Error checks. Run golangci-lint run ./... and
confirm no configured violations remain.
🪄 Autofix
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e747f629-e48f-4e23-833e-c85bd63ae3db
📒 Files selected for processing (3)
middleware/error_handling.gomiddleware/error_handling_regression_test.gomiddleware/logging.go
X-Lerian-Ref: 0x1 Co-authored-by: Codesmith <codesmith-bot@users.noreply.github.com>
What
errors.Asmatches a typed-nil*fiber.Errorinside a joined chain (errors.Join((*fiber.Error)(nil), ...)) and leaves the target nil. Reading.Codethen panics with a nil-pointer dereference, insideWithHTTPErrorHandling, the middleware whose purpose is closing exactly this defect class. Found by a downstream consumption review in Matcher (one of the six rows of its own typed-nil test table), confirmed by execution against the released version.Because
errors.Asstops at the first type match, a bare nil guard was not enough: a typed-nil*fiber.Errorjoined before a validfiber.NewError(400, ...)would shadow it and downgrade the mapped status to a generic 500. The fix is a shared extractor that mirrorserrors.Astraversal but skips nil matches and keeps searching, used both in the substitution branch ofWithHTTPErrorHandlingand inhttpStatusCode:Regression tests cover joined/delegating typed-nil shapes against Fiber's default ErrorHandler, including a typed-nil
*fiber.Errorpreceding a valid one (flat and nested in an earlier subtree).Affects both released lines: this PR fixes v3; the identical fix for the v2.1.x maintenance line is pushed on
fix/typed-nil-fiber-error-substitution-v2(cut from the v2.1.2 tag, needs av2.1.3tag on that branch, since main is now the /v3 module path).