fix(router): map middleware errors like handler errors - #35
Merged
Conversation
An error returned from middleware produced a 500 with a plain-text body,
while the same error returned from a handler produced its real status and
JSON envelope:
forge.Unauthorized("nope") from a handler -> 401 {"code":401,"error":"nope"}
forge.Unauthorized("nope") from middleware -> 500 "nope\n"
The typed-handler path calls handleError, which honours StatusCode() and
ResponseBody(). The three middleware paths in router_impl.go instead fell
back to http.Error whenever no ErrorHandler was installed — which is the
default — so every typed error lost its status on the way out.
The practical cost is that middleware cannot return errors at all. In
authsome, one consumer, all 19 middleware error responses are hand-written
ctx.JSON calls and none return a typed error, because returning one yields
a 500. That duplicates the envelope in every middleware and lets it drift
from the handler-side shape.
Use handleError in the fallback for all three sites, so an error carries
its status and envelope regardless of where it was returned. An installed
ErrorHandler still wins; only the default path changes. An untyped error
still maps to 500, so this promotes typed errors without swallowing
anything.
Tests assert the two paths produce identical status and body across
401/403/400/404, that the body is a JSON envelope rather than a text dump,
and that an untyped error stays a 500. All four fail without the change.
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Conventional Commits ValidationPR Title: valid |
Contributor
Conventional Commits ValidationPR Title: valid |
Contributor
Conventional Commits ValidationPR Title: valid |
juicycleff
added a commit
to xraph/authsome
that referenced
this pull request
Jul 31, 2026
Pulls in the middleware error-mapping fix (xraph/forge#35). Verified at runtime rather than by inspection: forge.Unauthorized now returns 401 {"code":401,"error":"nope"} from middleware, identical to the same error returned from a handler. It previously produced 500 "nope\n". Transitively bumps go-utils v1.1.1 => v1.1.2. Full suite green across 79 packages, golangci-lint 0 issues. Two things this release does NOT change, both deliberate: omitempty is still read as an optionality marker by isFieldRequired. Removing that inference would flip every such field from optional to required — 699 fields in this repo alone, breaking 77 tests across 15 packages, and rejecting previously-valid client requests. It needs a major version and a codemod. A required pointer in a request body is still not enforced: an absent `Name *string json:"name" required:"true"` returns 200. The nil check is gated on the field being a query/path/header parameter, so bodies are skipped, while the generated OpenAPI still advertises the field as required. That fix lives in go-utils (http/validator.go) and is not in v1.1.2 — so the middleware error responses in middleware/ must stay as hand-written ctx.JSON calls until it ships, and any required pointer in a request body is currently unenforced at runtime.
juicycleff
added a commit
to xraph/authsome
that referenced
this pull request
Jul 31, 2026
…elopes
Middleware could not return errors before forge v1.9.0: a typed error
returned from middleware lost its status and produced a 500 with a
plain-text body, so every rejection here was a hand-written ctx.JSON
call. That duplicated the {error, code} envelope 17 times and let it
drift from the handler-side shape, with nothing to catch the drift.
xraph/forge#35 fixed the mapping, so 14 of those become one-liners:
return ctx.JSON(http.StatusUnauthorized, map[string]any{
"error": "authentication required",
"code": http.StatusUnauthorized,
})
→
return forge.Unauthorized("authentication required")
Verified byte-identical before converting, by driving each constructor
through middleware and comparing envelopes: 401/403/400/404 and a 429
via NewHTTPError all produce exactly the payload the hand-written form
did. Response headers set before the return survive, so the rate-limit
X-RateLimit-* and Retry-After headers are unaffected.
Three sites deliberately stay hand-written, each commented in place:
- rbac.go ×2 — forge replaces the message on a 500 with a generic
"Internal Server Error", so converting would drop the specific
reason ("permission check failed", "role check failed") that
consumers currently receive.
- auth.go ×1 — the envelope carries a conditional debug_reason field,
which the typed constructors cannot express; they take a message
only.
Net 33 lines removed. Full suite green across 79 packages,
golangci-lint 0 issues.
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
1 participant
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.
An error returned from middleware produced a 500 with a plain-text body, while the same error returned from a handler produced its real status and JSON envelope:
Cause
handleErrorininternal/router/handler.goalready honoursStatusCode()andResponseBody(), but only the typed-handler path calls it. The three middleware paths inrouter_impl.go—applyMiddleware,applyMiddlewareAndInterceptors, andconvertForgeMiddlewareToHTTP— fall back tohttp.Errorwhenever noErrorHandleris installed, which is the default.So the correct mapping already existed; it just wasn't reachable from one of the two entry points. That's why this survived: the code reads as correct wherever you open it, and the asymmetry is only visible when the two paths are compared side by side.
Why it matters
Middleware effectively cannot return errors. In authsome — one consumer — all 19 middleware error responses are hand-written
ctx.JSONcalls, and none returns a typed error, because returning one yields a 500. Every middleware re-implements the envelope, and those copies drift from the handler-side shape over time.Change
Use
handleErrorin the fallback at all three sites.ErrorHandlerstill wins — only the default path changes.Tests
middleware_error_mapping_test.goasserts that middleware and handler paths produce identical status and body across 401/403/400/404, that the body is a JSON envelope rather than a text dump, and that an untyped error stays a 500.Verified load-bearing: reverting the three call sites fails 6 of them. Full
go test ./...is green, and authsome's full suite plusgolangci-lintare clean when pointed at this branch via a localreplace.