fix: clarify non-JSON 5xx responses as intermediary/gateway errors#32
Merged
Conversation
A prod ALB timed out a long-running request and returned an HTML 504 page. doMethod reported it as "flashduty: malformed response (http 504, request_id ): invalid character '<' looking for beginning of value", which reads like a Flashduty API JSON contract bug and led both humans and AI agents to misdiagnose it. Split the non-JSON-body failure by status class: a non-2xx response whose body isn't valid envelope JSON never came from the Flashduty API (it always answers in JSON), so name the intermediary, the missing/present request id, the likely gateway/proxy timeout cause, a retry/split hint, and a body snippet. 2xx non-JSON bodies (CSV/file exports) and non-2xx JSON error envelopes are unaffected.
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.
What broke in prod
Yesterday a long-running request hit an ALB in front of the Flashduty API. The ALB itself timed out the request (its own timeout, not the API's) and answered with an HTML 504 error page instead of forwarding the request through.
doMethodtried to parse that HTML as the JSON envelope, failed, and reported:That message reads exactly like a Flashduty API bug ("the API returned broken JSON"). Both a human and an AI agent looking at that error jumped to "the API has a contract bug" instead of the real cause: an intermediary cut the request off before the API ever answered.
Fix
Split the non-JSON-body failure in
doMethodby status class:Non-2xx + non-JSON body (the prod case): the Flashduty API always answers in JSON, so a non-JSON body on a failure status can only have come from an intermediary — gateway, load balancer, or proxy. New message names the status, says explicitly it's not from the Flashduty API, notes whether a request id was present, gives the actionable hint (retry / split long-running requests into smaller batches), and echoes a
%q-quoted snippet of the actual body so the real error page is visible:2xx + non-JSON body (CSV/file export endpoints): unchanged — still surfaced as
Response.Rawwith no error.Non-2xx + valid JSON error envelope: unchanged — still maps to
*ErrorResponseexactly as before.Tests
Added to
flashduty_test.go:TestDoReportsIntermediaryOnNon2xxNonJSONBody— 504 + HTML body + empty request-id header → new message, contains the status/intermediary/timeout hint/body snippet, does not contain "malformed response".TestDoReportsIntermediaryWithRequestID— same, but with a forwardedFlashcat-Request-Idheader, to confirm the id is included when present.TestDoMapsNon2xxJSONEnvelopeUnaffectedByGatewaySplit— 504 + valid JSON error envelope → still maps to*ErrorResponseunchanged.TestDoExposesNonJSONBodyAsRawdoc comment to note it also guards the 2xx branch against this split.Test plan
go build ./...go vet ./...go test -race ./...make lint(golangci-lint, 0 issues)make fmt(gofmt -s + gci) — no additional diffs beyond the change itself