Conversation
…xisting Status errors Wrap silently returns the original Status when err is already a Status error, which can be surprising. AlwaysWrap always applies the caller's code/reason/message while preserving existing details (field violations, metadata, etc.) via Clone. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR extends the statusx error-wrapping API with “always override” variants to ensure callers can enforce a new gRPC code/reason/message even when the input error is already a StatusError or gRPC status.Status, while preserving existing error details via FromError + Clone.
Changes:
- Add
AlwaysWrap/AlwaysWrapfto override code/reason/message for any non-nil error, while keeping the original error as the cause. - Add
AlwaysWrapCode/AlwaysWrapCodefconvenience helpers that derivereasonfrom the provided gRPC code. - Add comprehensive tests covering override behavior, detail preservation, immutability of the original
Status, and format-string variants.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
statusx/status.go |
Adds AlwaysWrap / AlwaysWrapf implementation that clones existing statuses and overrides code/reason/message. |
statusx/code.go |
Adds AlwaysWrapCode / AlwaysWrapCodef wrappers using ReasonFromCode. |
statusx/status_test.go |
Adds tests validating AlwaysWrap* behavior, including overrides and detail preservation. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Document nil-error behavior (returns OK status) for AlwaysWrap, AlwaysWrapCode, AlwaysWrapCodef - Document that localized key is reset to match the new reason when wrapping existing Status Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Clarify that computed Code()/Reason() follow the Status invariant where a non-nil cause cannot be OK, so passing codes.OK with a non-nil error results in codes.Unknown at runtime. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Also fixed variable shadowing (status -> st) in TestWrap for the same pattern. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
FromError already creates a fresh *Status for gRPC status errors and context errors. Only the *StatusError branch returns a shared pointer that needs cloning to avoid mutation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Locks in the documented behavior where Status invariant forces Code()/Reason() to Unknown when cause is non-nil but code is OK. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Mirror the statusx AlwaysWrap pattern for HTTP-based error handling: - AlwaysWrap / AlwaysWrapf: always override httpStatus/reason/message - AlwaysWrapStatus / AlwaysWrapStatusf: convenience variants with auto-derived reason - Comprehensive tests covering overrides, detail preservation, immutability, and edge cases Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // fields, even if err is already a Status error. The original error is preserved as | ||
| // the cause. If err is nil, it returns an OK status (consistent with Wrap). | ||
| // | ||
| // Note: the computed StatusCode()/Reason() follow the Status invariant that a non-error |
There was a problem hiding this comment.
The doc comment says StatusCode()/Reason() follow the invariant that a "non-error cause" cannot be OK. Here cause is an error, so "non-error cause" is confusing/incorrect wording. Consider rephrasing to something like "a non-nil cause cannot be OK" (or "a non-nil error cause").
| // Note: the computed StatusCode()/Reason() follow the Status invariant that a non-error | |
| // Note: the computed StatusCode()/Reason() follow the Status invariant that a non-nil |
Summary
AlwaysWrap/AlwaysWrapfthat always apply the caller's code/reason/message, even whenerris already a Status error (unlikeWrapwhich silently returns the original)AlwaysWrapCode/AlwaysWrapCodefconvenience variants that auto-derive reason from gRPC codeFromError+Cloneto preserve existing details (field violations, metadata, etc.) while overriding code/reason/messageTest plan
🤖 Generated with Claude Code