fix(api): improve error parsing for non-JSON API responses #12
Conversation
Add support for extracting error messages from text/plain and text/html responses, which can occur during gateway errors (502, 503) or when the API returns HTML error pages. Problem: - When Figma API returned non-JSON errors (e.g., 502 Bad Gateway with HTML), users would only see generic error message with no debugging context - No visibility into actual error content for troubleshooting Solution: - Check content-type header before parsing response body - For application/json: parse as JSON (existing behavior) - For text/plain or text/html: read body as text and use as error message - Truncate long responses (>200 chars) to prevent huge error messages - Gracefully handle empty bodies and parsing errors Changes: - src/api/fetcher.ts: Added text/plain and text/html response handling - src/api/mutator.ts: Added text/plain and text/html response handling - tests/api/fetcher.test.ts: Added 7 tests for text error responses - tests/api/mutator.test.ts: Added 7 tests for text error responses Test coverage: 100% Refs: Codex Audit Item #6
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #12 +/- ##
==========================================
+ Coverage 90.47% 90.73% +0.25%
==========================================
Files 34 34
Lines 903 917 +14
Branches 246 256 +10
==========================================
+ Hits 817 832 +15
+ Misses 85 84 -1
Partials 1 1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR improves error parsing for non-JSON API responses by adding support for text/plain and text/html content types, which commonly occur during gateway errors (502, 503) or when APIs return HTML error pages.
Key changes:
- Enhanced error message extraction to handle text/plain and text/html responses in addition to JSON
- Added truncation logic (200 characters) for long error responses to prevent unwieldy error messages
- Comprehensive test coverage for new error handling scenarios including empty bodies and parsing failures
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/api/fetcher.ts | Added text/plain and text/html error response handling with truncation logic |
| src/api/mutator.ts | Added text/plain and text/html error response handling with truncation logic |
| tests/api/fetcher.test.ts | Added 7 test cases covering text error responses, truncation, empty bodies, null content-type, and parsing errors |
| tests/api/mutator.test.ts | Added 7 test cases covering text error responses, truncation, empty bodies, null content-type, and parsing errors |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try { | ||
| await fetcher(DUMMY_URL, DUMMY_TOKEN) | ||
| } catch (err) { | ||
| expect((err as Error).message).toHaveLength(203) // 200 chars + "..." | ||
| expect((err as Error).message.endsWith('...')).toBe(true) | ||
| } | ||
| }) |
There was a problem hiding this comment.
The truncation test uses a try-catch block but doesn't verify that an error was actually thrown. If the function call unexpectedly succeeds, the test would pass without any assertions being executed. Add an assertion to ensure an error was thrown, such as expect.fail('Should have thrown') before the catch block or use expect.assertions(2) at the start of the test.
| try { | ||
| await mutator(url, token, 'CREATE', body) | ||
| } catch (err) { | ||
| expect((err as Error).message).toHaveLength(203) // 200 chars + "..." | ||
| expect((err as Error).message.endsWith('...')).toBe(true) | ||
| } | ||
| }) |
There was a problem hiding this comment.
The truncation test uses a try-catch block but doesn't verify that an error was actually thrown. If the function call unexpectedly succeeds, the test would pass without any assertions being executed. Add an assertion to ensure an error was thrown, such as expect.fail('Should have thrown') before the catch block or use expect.assertions(2) at the start of the test.
| } else if ( | ||
| contentType.includes('text/plain') || | ||
| contentType.includes('text/html') | ||
| ) { | ||
| // For text responses (e.g., 502 Bad Gateway), use the body text | ||
| const textBody = await response.text() | ||
| if (textBody) { | ||
| // Truncate long HTML/text responses to a reasonable length | ||
| const maxLength = 200 | ||
| errorMessage = | ||
| textBody.length > maxLength | ||
| ? `${textBody.slice(0, maxLength)}...` | ||
| : textBody | ||
| } |
There was a problem hiding this comment.
The error message extraction logic for text/plain and text/html responses is duplicated between fetcher.ts and mutator.ts (including the 200-character truncation logic). Consider extracting this into a shared utility function to reduce duplication and improve maintainability. This would make it easier to adjust the truncation length or error handling logic in the future without needing to update both files.
1 participant
Add support for extracting error messages from text/plain and text/html responses, which can occur during gateway errors (502, 503) or when the API returns HTML error pages.
Problem:
Solution:
Changes:
Test coverage: 100%
Refs: Codex Audit Item #6