Skip to content

Commit 42eeab1

Browse files
Include original response in client errors (#1476)
* Include original response in ClientError By including the original fetch response when returning a ClientError call sites can use the full response to debug why the error happened. This opens up for e.g. utilizing api's that might at any time return a non-graphql response when an error occurs. * Include headers in client error response types * refactor: expose headers and rawBody instead of Response object Instead of exposing the full fetch Response (whose body stream is already consumed), expose the useful data directly on GraphQLResponse: - headers: Headers - for accessing response headers like Kill-Switch - rawBody: string - the raw response text for non-GraphQL responses This avoids the "consumed body stream" footgun while still enabling the use case from issue #1473 (accessing non-GraphQL error responses). * test: update snapshots and assertions for body property Update tests to account for the new `body` property on GraphQLResponse: - Destructure `body` out of results in equality assertions - Update inline snapshots to include the body field * fix: add body to GraphQLClientResponse, fix jsdom test - Add body property to GraphQLClientResponse type (was only on GraphQLResponse) - Remove toBeInstanceOf(Headers) check that fails in jsdom environment --------- Co-authored-by: Jason Kuhrt <jasonkuhrt@me.com>
1 parent 599c487 commit 42eeab1

File tree

5 files changed

+57
-13
lines changed

5 files changed

+57
-13
lines changed

src/legacy/helpers/runRequest.ts

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -71,12 +71,16 @@ export const runRequest = async (input: Input): Promise<ClientError | GraphQLCli
7171
const fetcher = createFetcher(config.method)
7272
const fetchResponse = await fetcher(config)
7373

74+
// Read response body text first (can only be read once)
75+
const body = await fetchResponse.text()
76+
7477
// Parse response body FIRST, regardless of HTTP status
7578
// This allows GraphQL errors to be extracted even when HTTP status is 4xx/5xx (fixes #1281)
7679
let result
7780
try {
78-
result = await parseResultFromResponse(
79-
fetchResponse,
81+
result = parseResultFromText(
82+
body,
83+
fetchResponse.headers.get(CONTENT_TYPE_HEADER),
8084
input.fetchOptions.jsonSerializer ?? defaultJsonSerializer,
8185
)
8286
} catch (error) {
@@ -87,6 +91,7 @@ export const runRequest = async (input: Input): Promise<ClientError | GraphQLCli
8791
const clientResponseBase = {
8892
status: fetchResponse.status,
8993
headers: fetchResponse.headers,
94+
body,
9095
}
9196

9297
// Handle non-2xx HTTP status codes
@@ -159,9 +164,7 @@ const executionResultClientResponseFields = ($params: Input) => (executionResult
159164
}
160165
}
161166

162-
const parseResultFromResponse = async (response: Response, jsonSerializer: JsonSerializer) => {
163-
const contentType = response.headers.get(CONTENT_TYPE_HEADER)
164-
const text = await response.text()
167+
const parseResultFromText = (text: string, contentType: string | null, jsonSerializer: JsonSerializer) => {
165168
if (contentType && isGraphQLContentType(contentType)) {
166169
return parseGraphQLExecutionResult(jsonSerializer.parse(text))
167170
} else {

src/legacy/helpers/types.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ export interface GraphQLResponse<T = unknown> {
4040
errors?: GraphQLError[]
4141
extensions?: unknown
4242
status: number
43+
headers: Headers
44+
/**
45+
* The response body text. Useful for debugging non-GraphQL responses
46+
* (e.g., 401/403 errors that return plain JSON instead of GraphQL).
47+
*/
48+
body: string
4349
[key: string]: unknown
4450
}
4551

@@ -62,6 +68,11 @@ export type TypedDocumentString<$Result, $Variables> = String & DocumentTypeDeco
6268
export interface GraphQLClientResponse<Data> {
6369
status: number
6470
headers: Headers
71+
/**
72+
* The response body text. Useful for debugging non-GraphQL responses
73+
* (e.g., 401/403 errors that return plain JSON instead of GraphQL).
74+
*/
75+
body: string
6576
data: Data
6677
extensions?: unknown
6778
errors?: GraphQLError[]

tests/legacy/batching.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ test(`minimal double query`, async () => {
2323
test(`basic error`, async () => {
2424
mockServer.res({ body: [{ errors }] })
2525
await expect(batchRequests(mockServer.url, [{ document: `x` }])).rejects.toMatchInlineSnapshot(
26-
`[Error: GraphQL Error (Code: 200): {"response":{"0":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]}},"status":200,"headers":{}},"request":{"query":["x"],"variables":[null]}}]`,
26+
`[Error: GraphQL Error (Code: 200): {"response":{"0":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]}},"status":200,"headers":{},"body":"[{\\"errors\\":{\\"message\\":\\"Syntax Error GraphQL request (1:1) Unexpected Name \\\\\\"x\\\\\\"\\\\n\\\\n1: x\\\\n ^\\\\n\\",\\"locations\\":[{\\"line\\":1,\\"column\\":1}]}}]"},"request":{"query":["x"],"variables":[null]}}]`,
2727
)
2828
})
2929

@@ -34,7 +34,7 @@ test(`successful query with another which make an error`, async () => {
3434
await expect(
3535
batchRequests(mockServer.url, [{ document: `{ me { id } }` }, { document: `x` }]),
3636
).rejects.toMatchInlineSnapshot(
37-
`[Error: GraphQL Error (Code: 200): {"response":{"0":{"data":{"me":{"id":"some-id"}}},"1":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]}},"status":200,"headers":{}},"request":{"query":["{ me { id } }","x"],"variables":[null,null]}}]`,
37+
`[Error: GraphQL Error (Code: 200): {"response":{"0":{"data":{"me":{"id":"some-id"}}},"1":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]}},"status":200,"headers":{},"body":"[{\\"data\\":{\\"me\\":{\\"id\\":\\"some-id\\"}}},{\\"errors\\":{\\"message\\":\\"Syntax Error GraphQL request (1:1) Unexpected Name \\\\\\"x\\\\\\"\\\\n\\\\n1: x\\\\n ^\\\\n\\",\\"locations\\":[{\\"line\\":1,\\"column\\":1}]}}]"},"request":{"query":["{ me { id } }","x"],"variables":[null,null]}}]`,
3838
)
3939
})
4040

tests/legacy/general.test.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,12 @@ test(`minimal query`, async () => {
1515

1616
test(`minimal raw query`, async () => {
1717
const mockRes = ctx.res({ body: { data, extensions: { version: `1` } } }).spec.body!
18-
const { headers: _, ...result } = await rawRequest(ctx.url, `{ me { id } }`)
18+
const { headers: _, body: __, ...result } = await rawRequest(ctx.url, `{ me { id } }`)
1919
expect(result).toEqual({ data: mockRes.data, extensions: mockRes.extensions, status: 200 })
2020
})
2121

2222
test(`minimal raw query with response headers`, async () => {
23-
const { headers: reqHeaders, body } = ctx.res({
23+
const { headers: reqHeaders, body: mockBody } = ctx.res({
2424
headers: {
2525
'Content-Type': `application/json`,
2626
'X-Custom-Header': `test-custom-header`,
@@ -31,8 +31,8 @@ test(`minimal raw query with response headers`, async () => {
3131
},
3232
}).spec
3333

34-
const { headers, ...result } = await rawRequest(ctx.url, `{ me { id } }`)
35-
expect(result).toEqual({ ...body, status: 200 })
34+
const { headers, body: _, ...result } = await rawRequest(ctx.url, `{ me { id } }`)
35+
expect(result).toEqual({ ...mockBody, status: 200 })
3636
expect(headers.get(`X-Custom-Header`)).toEqual(reqHeaders![`X-Custom-Header`])
3737
})
3838

@@ -41,15 +41,15 @@ test(`basic error`, async () => {
4141
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
4242
const res = await request(ctx.url, `x`).catch((x: unknown) => x)
4343
expect(res).toMatchInlineSnapshot(
44-
`[Error: GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{}},"request":{"query":"x"}}]`,
44+
`[Error: GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{},"body":"{\\"errors\\":{\\"message\\":\\"Syntax Error GraphQL request (1:1) Unexpected Name \\\\\\"x\\\\\\"\\\\n\\\\n1: x\\\\n ^\\\\n\\",\\"locations\\":[{\\"line\\":1,\\"column\\":1}]}}"},"request":{"query":"x"}}]`,
4545
)
4646
})
4747

4848
test(`basic error with raw request`, async () => {
4949
ctx.res({ body: { errors } })
5050
const res: unknown = await rawRequest(ctx.url, `x`).catch((x: unknown) => x)
5151
expect(res).toMatchInlineSnapshot(
52-
`[Error: GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{}},"request":{"query":"x"}}]`,
52+
`[Error: GraphQL Error (Code: 200): {"response":{"errors":{"message":"Syntax Error GraphQL request (1:1) Unexpected Name \\"x\\"\\n\\n1: x\\n ^\\n","locations":[{"line":1,"column":1}]},"status":200,"headers":{},"body":"{\\"errors\\":{\\"message\\":\\"Syntax Error GraphQL request (1:1) Unexpected Name \\\\\\"x\\\\\\"\\\\n\\\\n1: x\\\\n ^\\\\n\\",\\"locations\\":[{\\"line\\":1,\\"column\\":1}]}}"},"request":{"query":"x"}}]`,
5353
)
5454
})
5555

tests/legacy/http-status-with-errors.test.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,34 @@ describe(`HTTP 4xx/5xx status codes with GraphQL response body`, () => {
155155
expect(clientError.response.errors).toEqual(graphqlErrors)
156156
}
157157
})
158+
159+
test(`non-GraphQL response - headers and rawBody should be accessible`, async () => {
160+
const nonGraphQLBody = { error: `Kill switch activated`, reason: `maintenance` }
161+
162+
// Setup mock to return 503 with non-GraphQL response and custom header
163+
// eslint-disable-next-line prefer-arrow/prefer-arrow-functions
164+
ctx.server.use(`*`, function mock(req, res) {
165+
res.status(503)
166+
.set(`Kill-Switch`, `true`)
167+
.set(`Content-Type`, `application/json`)
168+
.send(JSON.stringify(nonGraphQLBody))
169+
})
170+
171+
const client = new GraphQLClient(ctx.url)
172+
173+
try {
174+
await client.request(`{ user { id } }`)
175+
expect.fail(`Expected ClientError to be thrown`)
176+
} catch (error) {
177+
expect(error).toBeInstanceOf(ClientError)
178+
const clientError = error as ClientError
179+
expect(clientError.response.status).toBe(503)
180+
// Verify headers are accessible
181+
expect(clientError.response.headers.get(`Kill-Switch`)).toBe(`true`)
182+
// Verify body is accessible for non-GraphQL responses
183+
expect(clientError.response.body).toBe(JSON.stringify(nonGraphQLBody))
184+
// Can parse the body to get detailed error info
185+
expect(JSON.parse(clientError.response.body)).toEqual(nonGraphQLBody)
186+
}
187+
})
158188
})

0 commit comments

Comments
 (0)