fix([cos-runner-client-non-json-response-handling]): tolerate non-JSON runner responses#610
Merged
atomantic merged 3 commits intoJun 1, 2026
Conversation
…N runner responses
The cosRunnerClient fetch wrappers called response.json() unconditionally on
every response. When the runner returns an HTML error page instead of JSON
(e.g. a 500 while PM2 is restarting it mid-request), the parse threw
"Unexpected token <" and masked the runner's actual error.
Route all body parsing through a tolerant readRunnerJson() helper that reads
the raw text and falls back to { error: <raw text> } on a parse failure, so
callers surface the runner's real message.
…N.md and log to changelog
…SONParse for tolerant runner parsing
/simplify reuse pass: readRunnerJson re-implemented the tolerant-parse pattern
that server/lib/fileUtils.js#safeJSONParse already provides. Delegate to it,
keeping the empty-body -> {} guard so spreading callers don't pick up a
spurious error key. Capture the broader 'adopt a shared tolerant fetch->JSON
helper across the other fetch clients' cleanup in PLAN.md (out of this PR's
single-file scope).
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
The CoS runner client (
server/services/cosRunnerClient.js) calledresponse.json()unconditionally on every runner response — both success and error paths. When the runner returns an HTML body instead of JSON (e.g. a 500 error page while PM2 is restarting it mid-request), the parse threwUnexpected token <and masked the runner's actual error.All ~20 parse sites now route through a small
readRunnerJson(response)helper that reads the raw text and parses it tolerantly via the existingsafeJSONParse(server/lib/fileUtils.js), falling back to{ error: <raw text> }on a non-JSON body so callers surface the runner's real message. An empty body returns{}(an empty success, distinct from a parse failure) so spreading callers likegetRunnerHealthdon't pick up a spuriouserrorkey.A
/simplifyreuse pass replaced an initial hand-rolled try/catch with the cataloguedsafeJSONParse. The broader cleanup — promoting a shared tolerant fetch→JSON helper and adopting it across the other unguarded fetch clients (aiProvider.js,browserService.js, etc.) — is captured in PLAN.md as[shared-tolerant-fetch-json-helper], out of this single-file PR's scope.Test plan
cd server && npm test -- cosRunnerClient— 39 tests pass (36 existing + 3 new).502 Bad Gateway), an empty error body falling back to the default message, and a non-JSON success body not crashing.mockResponsehelper was updated to serve the body viatext()(objects serialized, strings passed raw) to match the new read path.