Handle 200-with-empty-body responses in shared HTTP client#12
Open
andreibondarev wants to merge 1 commit into
Open
Handle 200-with-empty-body responses in shared HTTP client#12andreibondarev wants to merge 1 commit into
andreibondarev wants to merge 1 commit into
Conversation
Some upstream APIs (e.g. DOL OSHA when no rows match the filter) return HTTP 200 with an empty body. The client called `await res.json()` directly, which throws "Unexpected end of JSON input" and bubbles out of tool execute() as a confusing error instead of an empty result. This reads the body as text first, returns null for empty/ whitespace-only responses, and wraps JSON.parse with a clearer error message that includes a body preview when the response is non-JSON. Adds vitest coverage for the four cases (empty, whitespace, non-JSON, valid JSON). Reproduced via: dol_osha_accidents with event_keyword=fentanyl.
Author
|
@lzinga Thank you for building this. |
There was a problem hiding this comment.
Pull request overview
This PR hardens the shared createClient HTTP client so that upstream APIs returning HTTP 200 with an empty (or whitespace-only) body no longer cause a JSON parse crash, and adds unit tests to validate the new behavior.
Changes:
- Update
createClientto read successful responses as text first, returningnullfor empty/whitespace-only bodies and emitting a clearer error for non-JSON bodies. - Add vitest coverage for empty body, whitespace-only body, non-JSON body, and valid JSON body scenarios.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/shared/client.ts |
Adjusts the success-path body parsing to handle empty bodies safely and improve parse error diagnostics. |
tests/client-empty-body.test.ts |
Adds unit tests covering the new empty/invalid body parsing behavior in the shared client. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+10
to
+19
| import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; | ||
| import { createClient } from "../src/shared/client.js"; | ||
|
|
||
| describe("createClient — empty/invalid body handling", () => { | ||
| const originalFetch = globalThis.fetch; | ||
|
|
||
| beforeEach(() => { | ||
| // Keep cache effects out of these tests by varying the URL per case. | ||
| }); | ||
|
|
Comment on lines
+30
to
+35
| const client = createClient({ | ||
| baseUrl: "https://example.test", | ||
| name: "empty-body-test", | ||
| }); | ||
|
|
||
| const data = await client.get<unknown>("/empty"); |
Comment on lines
+538
to
+542
| const raw = await res.text(); | ||
| let data: unknown; | ||
| if (raw.trim() === "") { | ||
| data = null; | ||
| } else { |
Comment on lines
+547
to
+548
| throw new Error( | ||
| `${name}: invalid JSON response (HTTP ${res.status}): ${(err as Error).message}. Body: ${preview}`, |
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.
Description
Some upstream APIs return HTTP 200 with an empty body when there are no rows matching the filter. The shared HTTP client called
await res.json()directly, which throwsSyntaxError: Unexpected end of JSON input. That bubbles out oftool.execute()as a confusing error instead of an empty result.Reproduced via
dol_osha_accidentswithevent_keyword: "keyword":The companion tool
dol_osha_inspectionshandles "no results" cleanly (returns the standardemptyResponse), so the inconsistency is purely in the client's body-parsing path.This change reads the body as text first in
src/shared/client.ts:null(the existingArray.isArray(data) || !data.lengthchecks in tool wrappers already handle this and emitemptyResponse).Unexpected end of JSON input.Adds vitest coverage for all four cases (empty, whitespace, non-JSON, valid JSON).
Type of Change
Verification
npm run build— passesnpm test— 926/926 passed (922 existing + 4 new)Additional Notes
The fix is in shared client code, so it benefits every module that uses
createClient(FRED, DOL, BLS, etc.) — not just DOL. If any other API silently returns empty bodies, those will now degrade toemptyResponseinstead of throwing.I considered making the JSON-parse error a soft warning +
nullrather than a throw, but the current behavior (rejecting non-JSON 200s as errors) is probably what most callers want, and changing that would be a behavior change beyond the scope of this fix.