Skip to content

Commit 9809932

Browse files
committed
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).
1 parent fb0a9a2 commit 9809932

File tree

4 files changed

+47
-11
lines changed

4 files changed

+47
-11
lines changed

src/legacy/classes/ClientError.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,8 @@ import type { GraphQLRequestContext, GraphQLResponse } from '../helpers/types.js
33
export class ClientError extends Error {
44
public response: GraphQLResponse
55
public request: GraphQLRequestContext
6-
public originalResponse: Response
76

8-
constructor(response: GraphQLResponse, request: GraphQLRequestContext, originalResponse: Response) {
7+
constructor(response: GraphQLResponse, request: GraphQLRequestContext) {
98
const message = `${ClientError.extractMessage(response)}: ${
109
JSON.stringify({
1110
response,
@@ -19,7 +18,6 @@ export class ClientError extends Error {
1918

2019
this.response = response
2120
this.request = request
22-
this.originalResponse = originalResponse
2321

2422
// this is needed as Safari doesn't support .captureStackTrace
2523
if (typeof Error.captureStackTrace === `function`) {

src/legacy/helpers/runRequest.ts

Lines changed: 10 additions & 8 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 rawBody = 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+
rawBody,
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+
rawBody,
9095
}
9196

9297
// Handle non-2xx HTTP status codes
@@ -100,7 +105,6 @@ export const runRequest = async (input: Input): Promise<ClientError | GraphQLCli
100105
query: input.request._tag === `Single` ? input.request.document.expression : input.request.query,
101106
variables: input.request.variables,
102107
},
103-
fetchResponse,
104108
)
105109
}
106110

@@ -115,7 +119,7 @@ export const runRequest = async (input: Input): Promise<ClientError | GraphQLCli
115119
return new ClientError(clientResponse, {
116120
query: input.request._tag === `Single` ? input.request.document.expression : input.request.query,
117121
variables: input.request.variables,
118-
}, fetchResponse)
122+
})
119123
}
120124

121125
// For 2xx responses, parse errors should throw
@@ -133,7 +137,7 @@ export const runRequest = async (input: Input): Promise<ClientError | GraphQLCli
133137
return new ClientError(clientResponse, {
134138
query: input.request._tag === `Single` ? input.request.document.expression : input.request.query,
135139
variables: input.request.variables,
136-
}, fetchResponse)
140+
})
137141
}
138142
switch (result._tag) {
139143
case `Single`:
@@ -160,9 +164,7 @@ const executionResultClientResponseFields = ($params: Input) => (executionResult
160164
}
161165
}
162166

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

src/legacy/helpers/types.ts

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

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -155,4 +155,35 @@ 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).toBeInstanceOf(Headers)
182+
expect(clientError.response.headers.get(`Kill-Switch`)).toBe(`true`)
183+
// Verify rawBody is accessible for non-GraphQL responses
184+
expect(clientError.response.rawBody).toBe(JSON.stringify(nonGraphQLBody))
185+
// Can parse the raw body to get detailed error info
186+
expect(JSON.parse(clientError.response.rawBody)).toEqual(nonGraphQLBody)
187+
}
188+
})
158189
})

0 commit comments

Comments
 (0)