feature: Include gRPC status details in error log messages#148
Conversation
The unary interceptor logged failed calls with only the raw error string, dropping the structured google.rpc.Status details (BadRequest, ErrorInfo, etc.) that explain the failure. Add a details field that renders every status detail compactly (type + one-line value) via a small statusDetails helper covered by unit tests. Ref: roadrunner-server/roadrunner#1897
📝 WalkthroughWalkthroughAdds a statusDetails helper that formats only google.rpc.* protobuf details from grpc/status errors and updates the gRPC interceptor to include those rendered details in structured error logs; tests exercise empty, non-status, single, multiple, non-google.rpc, and mixed-detail scenarios. ChangesgRPC error detail logging
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Pull request overview
This PR enhances the gRPC unary server interceptor’s error logging by preserving and emitting structured google.rpc.Status detail messages (e.g., google.rpc.BadRequest field violations) alongside the primary error, improving debuggability of failed RPCs.
Changes:
- Add a
statusDetails(*status.Status) []stringhelper to render gRPC statusDetails()into compact log-friendly strings. - Extend interceptor error logging to include a
detailsfield when status details are present. - Introduce unit tests covering no-details, single-detail, and multiple-detail formatting scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| server.go | Adds statusDetails and includes its output in interceptor error logs under a details field. |
| server_test.go | Adds unit tests validating statusDetails behavior for empty, single, and multiple details. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #148 +/- ##
==========================================
+ Coverage 77.56% 78.38% +0.81%
==========================================
Files 7 7
Lines 468 495 +27
==========================================
+ Hits 363 388 +25
- Misses 79 80 +1
- Partials 26 27 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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 `@server.go`:
- Around line 135-142: The loop that serializes decoded Any details (variables:
details, out, d, m) should be restricted to only the well-known google.rpc
error-detail types; update the body that currently checks d.(proto.Message) to
instead check m.ProtoReflect().Descriptor().FullName() against an allowlist of
the standard types (e.g., google.rpc.BadRequest, google.rpc.DebugInfo,
google.rpc.ErrorInfo, google.rpc.LocalizedMessage, google.rpc.QuotaFailure,
google.rpc.RetryInfo, etc.), and only call prototext.MarshalOptions{}.Format(m)
for those allowed names; for any other message types append a safe placeholder
like "<redacted>" or just the message full name (e.g., fmt.Sprintf("%s:
<redacted>", fullName)) to out. Ensure you preserve the existing variables
(details, out, m) and only change the conditional/serialization logic in the
loop.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
A handler can attach an arbitrarily large custom detail via Any; serializing it into the error log on every failed call is a needless cost. Log only the well-known google.rpc.* error details (BadRequest, ErrorInfo, ...), which carry the useful diagnostics, and skip anything else. Addresses CodeRabbit review on #148. Ref: roadrunner-server/roadrunner#1897
Replace the FullName-prefix check with a type switch over the errdetails types, so skipped (potentially large) details aren't touched by reflection at all. Ref: roadrunner-server/roadrunner#1897
The gRPC unary interceptor logged failed calls with only the raw error string, dropping the structured
google.rpc.Statusdetails that explain the failure. The error log now includes adetailsfield rendering each status detail compactly (type + one-line value) — e.g.google.rpc.BadRequest: field_violations:{field:"email" description:"is required"}.closes roadrunner-server/roadrunner#1897
Summary by CodeRabbit
Tests
Chores