fix(sdk): handle non-JSON error responses, add HTTP retry, defensive null checks#1734
Open
DukeDeSouth wants to merge 1 commit intoupstash:masterfrom
Open
fix(sdk): handle non-JSON error responses, add HTTP retry, defensive null checks#1734DukeDeSouth wants to merge 1 commit intoupstash:masterfrom
DukeDeSouth wants to merge 1 commit intoupstash:masterfrom
Conversation
…null checks **Problem:** 1. `HttpClient.request()` crashes with `SyntaxError` when the server returns non-JSON error responses (HTML from proxies, plain text). The code called `res.json()` without catching parse failures, so users got a confusing `SyntaxError` instead of a meaningful `Context7Error`. 2. Only network-level `fetch` failures were retried. HTTP 429 (rate limit) and 5xx (server errors) were not retried, even though the client is configured with exponential backoff and up to 5 retries. 3. `SearchLibraryCommand` and `GetContextCommand` assumed API response fields (`results`, `codeSnippets`, `infoSnippets`) were always present. If the API returned an incomplete response, the commands crashed with `Cannot read properties of undefined (reading 'map')`. 4. `retry: false` incorrectly set `attempts: 1` (2 total tries) instead of `attempts: 0` (1 total try, no retries). **Fix:** 1. Read error body once as text, then attempt `JSON.parse`. Long non-JSON bodies (>200 chars) fall back to `statusText (status)`. Short text bodies are used directly as the error message. 2. Added retry logic for HTTP 429, 500, 502, 503, 504. Respects `Retry-After` header on 429 responses. Uses the configured exponential backoff function as fallback. 3. Added nullish coalescing (`?? []`) for array fields from API responses, and an explicit `Array.isArray` guard for search results. 4. Fixed `retry: false` to set `attempts: 0`. **Tests:** 39 new unit tests covering all error handling paths, retry scenarios (network + HTTP status), defensive parsing, and edge cases. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Human View
Summary
Fixes three related robustness issues in the
@upstash/context7SDK's HTTP client and command layer:Bug 1:
SyntaxErrorcrash on non-JSON error responsesBefore: When the Context7 API (or an intermediary proxy) returned a non-JSON error response (e.g., HTML
502 Bad Gatewaypage, plain text),HttpClient.request()calledres.json()which threw an uncaughtSyntaxError. Users saw a confusingSyntaxError: Unexpected token < in JSON at position 0instead of a meaningfulContext7Error.After: The error body is read once as text via
res.text(), thenJSON.parseis attempted. If parsing fails, short text bodies (<=200 chars) are used directly as the error message; long bodies (HTML pages) fall back tostatusText (status). The error is always wrapped in aContext7Error.Bug 2: No retry on HTTP 429/5xx status codes
Before: The retry loop only caught network-level
fetchfailures (DNS errors, connection refused, etc.). Server errors (500, 502, 503, 504) and rate limits (429) were not retried, even though the client was configured with exponential backoff and up to 5 retries. This is especially impactful because Context7's own MCP server returns 429 for rate-limited requests.After: HTTP 429, 500, 502, 503, and 504 responses are now retried with the configured backoff strategy. The
Retry-Afterheader is respected on 429 responses. Non-retryable client errors (400, 401, 404) are still immediately thrown.Bug 3: Crash on incomplete API response fields
Before:
SearchLibraryCommand.exec()assumedresult.resultswas always a valid array, andGetContextCommand.exec()assumedapiResult.codeSnippetsandapiResult.infoSnippetswere always present. Missing or null fields causedTypeError: Cannot read properties of undefined (reading 'map').After: Added nullish coalescing for array fields and an
Array.isArrayguard for search results. Missing fields now gracefully return empty arrays.Additional fix:
retry: falsecreated 2 attempts instead of 1The constructor set
attempts: 1forretry: false, but the loop conditioni <= attemptsproduced 2 iterations. Fixed toattempts: 0(1 total attempt, 0 retries).Test plan
Added 39 new unit tests (mocked
fetch, no API key needed):All 39 tests pass locally.
AI View (DCCE Protocol v1.0)
Metadata
AI Contribution Summary
Verification Steps Performed
Human Review Guidance
JSON.parse,result.resultsMade with M7 Cursor