Skip to content

fix(api): improve error parsing for non-JSON API responses #12

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marklearst merged 1 commit into from
Dec 30, 2025
Merged

fix(api): improve error parsing for non-JSON API responses #12

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 18 additions & 4 deletions src/api/fetcher.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,19 +141,33 @@ export async function fetcher<TResponse = unknown>(
}
}

// Try to extract error message from JSON response
// Try to extract error message from response body
try {
const contentType = response.headers.get('content-type')
if (contentType?.includes('application/json')) {
const contentType = response.headers.get('content-type') ?? ''
if (contentType.includes('application/json')) {
const errorData = await response.json()
if (errorData?.message) {
errorMessage = errorData.message
} else if (errorData?.err) {
errorMessage = errorData.err
}
} 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
}
}
} catch {
// Ignore JSON parse errors, use default message
// Ignore parse errors, use default message
}

throw new FigmaApiError(errorMessage, statusCode, retryAfter)
Expand Down
22 changes: 18 additions & 4 deletions src/api/mutator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,29 @@ export async function mutator<TResponse = unknown>(
}
}

// Try to extract error message from JSON response
// Try to extract error message from response body
try {
const contentType = response.headers.get('content-type')
if (contentType?.includes('application/json')) {
const contentType = response.headers.get('content-type') ?? ''
if (contentType.includes('application/json')) {
const errorData = await response.json()
errorMessage = errorData.err || errorData.message || errorMessage
} 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
}
Comment on lines +162 to +175
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.
}
} catch {
// Ignore JSON parse errors, use default message
// Ignore parse errors, use default message
}

throw new FigmaApiError(errorMessage, statusCode, retryAfter)
Expand Down
112 changes: 112 additions & 0 deletions tests/api/fetcher.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,118 @@ describe('fetcher', () => {
)
})

it('should extract error message from text/plain response', async () => {
const mockHeaders = new Headers()
mockHeaders.set('content-type', 'text/plain')

mockFetch(
{
status: 503,
text: () => Promise.resolve('Service Unavailable'),
headers: mockHeaders,
},
false
)

await expect(fetcher(DUMMY_URL, DUMMY_TOKEN)).rejects.toThrow(
'Service Unavailable'
)
})

it('should extract error message from text/html response', async () => {
const mockHeaders = new Headers()
mockHeaders.set('content-type', 'text/html')

mockFetch(
{
status: 502,
text: () => Promise.resolve('<html><body>Bad Gateway</body></html>'),
headers: mockHeaders,
},
false
)

await expect(fetcher(DUMMY_URL, DUMMY_TOKEN)).rejects.toThrow(
'<html><body>Bad Gateway</body></html>'
)
})

it('should truncate long text error responses', async () => {
const mockHeaders = new Headers()
mockHeaders.set('content-type', 'text/html')

const longHtml = '<html>' + 'x'.repeat(300) + '</html>'

mockFetch(
{
status: 502,
text: () => Promise.resolve(longHtml),
headers: mockHeaders,
},
false
)

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)
}
})
Comment on lines +431 to +437
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

it('should use default message when text body is empty', async () => {
const mockHeaders = new Headers()
mockHeaders.set('content-type', 'text/plain')

mockFetch(
{
status: 503,
text: () => Promise.resolve(''),
headers: mockHeaders,
},
false
)

await expect(fetcher(DUMMY_URL, DUMMY_TOKEN)).rejects.toThrow(
'An error occurred while fetching data from the Figma API'
)
})

it('should use default message when content-type is null', async () => {
const mockHeaders = new Headers()
// No content-type set

mockFetch(
{
status: 500,
headers: mockHeaders,
},
false
)

await expect(fetcher(DUMMY_URL, DUMMY_TOKEN)).rejects.toThrow(
'An error occurred while fetching data from the Figma API'
)
})

it('should use default message when text() throws', async () => {
const mockHeaders = new Headers()
mockHeaders.set('content-type', 'text/plain')

mockFetch(
{
status: 503,
text: () => Promise.reject(new Error('Read error')),
headers: mockHeaders,
},
false
)

await expect(fetcher(DUMMY_URL, DUMMY_TOKEN)).rejects.toThrow(
'An error occurred while fetching data from the Figma API'
)
})

it('should not parse Retry-After for non-429 errors', async () => {
const mockHeaders = new Headers()
mockHeaders.set('content-type', 'application/json')
Expand Down
87 changes: 87 additions & 0 deletions tests/api/mutator.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,93 @@ describe('mutator', () => {
)
})

it('should extract error message from text/plain response', async () => {
mockFetch.mockResolvedValue({
ok: false,
status: 503,
headers: {
get: (name: string) => (name === 'content-type' ? 'text/plain' : null),
},
text: () => Promise.resolve('Service Unavailable'),
})
await expect(mutator(url, token, 'CREATE', body)).rejects.toThrow(
'Service Unavailable'
)
})

it('should extract error message from text/html response', async () => {
mockFetch.mockResolvedValue({
ok: false,
status: 502,
headers: {
get: (name: string) => (name === 'content-type' ? 'text/html' : null),
},
text: () => Promise.resolve('<html><body>Bad Gateway</body></html>'),
})
await expect(mutator(url, token, 'CREATE', body)).rejects.toThrow(
'<html><body>Bad Gateway</body></html>'
)
})

it('should truncate long text error responses', async () => {
const longHtml = '<html>' + 'x'.repeat(300) + '</html>'
mockFetch.mockResolvedValue({
ok: false,
status: 502,
headers: {
get: (name: string) => (name === 'content-type' ? 'text/html' : null),
},
text: () => Promise.resolve(longHtml),
})
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)
}
})
Comment on lines +214 to +220
Copy link

Copilot AI Dec 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copilot uses AI. Check for mistakes.

it('should use default message when text body is empty', async () => {
mockFetch.mockResolvedValue({
ok: false,
status: 503,
headers: {
get: (name: string) => (name === 'content-type' ? 'text/plain' : null),
},
text: () => Promise.resolve(''),
})
await expect(mutator(url, token, 'CREATE', body)).rejects.toThrow(
'An API error occurred'
)
})

it('should use default message when content-type is null', async () => {
mockFetch.mockResolvedValue({
ok: false,
status: 500,
headers: {
get: () => null,
},
})
await expect(mutator(url, token, 'CREATE', body)).rejects.toThrow(
'An API error occurred'
)
})

it('should use default message when text() throws', async () => {
mockFetch.mockResolvedValue({
ok: false,
status: 503,
headers: {
get: (name: string) => (name === 'content-type' ? 'text/plain' : null),
},
text: () => Promise.reject(new Error('Read error')),
})
await expect(mutator(url, token, 'CREATE', body)).rejects.toThrow(
'An API error occurred'
)
})

it('should use default errorMessage when both err and message are falsy', async () => {
mockFetch.mockResolvedValue({
ok: false,
Expand Down