fix(server): answer every process-generated error with the envelope - #871
Merged
Merged
Conversation
Three responses the process generated were text/plain: an unmatched /api/ path (404), a method the route does not accept (405), and a parameter that fails to bind before the handler runs, where the generated router's default wrote the parser's own error text, rejected value included. Three more sat in the SSE handler before the stream starts. A client that parses every non-2xx as the envelope failed on all of them and could not tell an OpenWatch 404 from a proxy's. Contract first: system-http-server 1.5.0 adds C-15 (every 4xx or 5xx the process generates carries the envelope; a parameter message names the parameter and never echoes the value; intermediary bodies are out of scope and clients must tolerate them) and AC-20. api-events-stream 1.1.1 restates AC-02 for the envelope. The router is mounted with an ErrorHandlerFunc that maps the generated error types to request.missing_parameter or request.invalid_parameter; the SPA fallback and the 405 handler use writeError; the SSE handler's three early exits use it too. AC-20 exercises the three cases end to end (status, Content-Type, X-Correlation-Id, code, message naming the parameter and not the value) and walks internal/server for any bare http.Error call so a new plain-text error fails the criterion. The API guide's error section states the behavior and keeps the client guidance for non-JSON infrastructure errors. CP: bugs/doing/OW-063
github-actions
Bot
added
documentation
remyluslosius
added a commit
that referenced
this pull request
Sep 21, 2026
remyluslosius
added a commit
that referenced
this pull request
Sep 22, 2026
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.
Summary
CP
bugs/doing/OW-063. Contract first, then code, then the guide.Contract.
system-http-server1.5.0 adds C-15: every 4xx or 5xx the OpenWatch process generates carries the canonical envelope, including the responses no handler produces (unmatched/api/path 404, method not allowed 405, parameter binding 400); a parameter message names the parameter and never echoes the rejected value or parser text; intermediary bodies are out of scope and clients must tolerate them. AC-20 covers the three cases plus a structural walk.api-events-stream1.1.1 restates AC-02 for the envelope (the SSE handler's early 400 was plain text).Code.
request_errors.go: anErrorHandlerFuncfor the generated router mappingRequiredParamError,RequiredHeaderError,InvalidParamFormatError,TooManyValuesForParamError,UnescapedCookieParamErrorandUnmarshalingParamErrortorequest.missing_parameterorrequest.invalid_parameter; the message carries the parameter name only.server.gomounts the router withHandlerWithOptionsand that handler; the 405 handler useswriteError.spa.go: the/api/fallthrough answersrequest.not_found; the non-GET branch answersrequest.method_not_allowed.sse_handler.go: the three pre-stream exits usewriteError(server.unavailable,request.missing_parameter,server.internal).Tests.
TestRequestErrors_EnvelopeEverywhere(AC-20):GET /api/v1/definitely-not-a-route404,DELETE /api/v1/health405,GET /api/v1/audit/events?limit=abc400 naminglimitand not containingabc; each withapplication/jsonandX-Correlation-Id. Plus a source walk that fails on any barehttp.Errorininternal/serveroutside tests and generated code.sse_handler_test.goAC-02 updated to the envelope.Guide. The error section states the behavior and keeps the client guidance for non-JSON infrastructure errors. Merge after #870 (OW-065), whose sentence about the three plain-text cases this paragraph supersedes; I will refresh on conflict.
Checks
go test ./internal/server/green (165 s, dedicated test DB);make spec-check121/121, 100%; doc style clean; pre-commit and pre-push hooks passed.Candidate impact
v0.8.0-rc.5is immutable and carries the old behavior; nothing here changes it.