refactor(go): shared apierr error renderer - #239
Merged
Merged
Conversation
…ses duplicate writeErrs (rest/webauthn), hydra error maps, DCR errors, and the gate's raw 401 into three renderers; fixes /mcp allowlist 403 to application/json (Node parity) and normalizes the unreachable defensive 401s in ws/agent-proxy/step-up to the {error} shape
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Builds the shared error-rendering package the architecture doc planned but never shipped, while the goldens still pin every shape.
internal/apierrnow owns the three error envelopes in the wire contract:apierr.Write→{error}(standard envelope)apierr.WriteCode→{error, code}(step-up gate, contract item F)apierr.WriteOAuth→{error, error_description}(mediated DCR)Every surface renders through it:
rest.writeErrandwebauthn.writeErr(previously duplicate implementations) delegate; hydra's 16 error-mapwriteJSONStatuscalls and the 4writeDCRErrcalls are migrated (helper deleted); the bearer gate's hand-concatenated{"error":...}bytes go through the renderer (keeping its WWW-Authenticate logic). Per-surfacewriteJSONhelpers keep success payloads only — nohttp.Erroror hand-rolled error body remains outside tests.The point (per the review that motivated this): the post-cutover envelope convergence — folding
{error}into{error, code}, item F — becomes a change to one package instead of a hunt through five.Deliberate behavior changes (all verified against Node)
/mcpIP-allowlist 403 was JSON-in-text/plainviahttp.Error; Node sendsapplication/json(ip-allowlist.ts:94). Nowapplication/json— a parity fix, body unchanged.text/plainJSON. All are unreachable through the real mux — the bearer gate runs first — and have no Node equivalent. Normalized to the{error}envelope and annotated as defensive.json.Encoderemits (every other envelope already had it). Goldens parse JSON, so nothing pinned changes.Tests
internal/apierr/apierr_test.gopins the exact bytes (body, status, content-type) of all three envelopes, including the hydra{error}-with-200 case.go vet,pnpm spdx:check.Also updates
docs/go-backend-architecture.md§5.5, which claimed the unified renderer "was never needed" and undercounted the render paths (it missed the two duplicate writeErrs and the text stragglers).