Skip to content

Commit

Permalink
Fix type signature of ServerError (#10810)
Browse files Browse the repository at this point in the history
* Update throwServerError.ts

`result` can be a string

* chore: add test case and fix type error in persisted query link

* chore: add changeset

---------

Co-authored-by: alessia <alessia@apollographql.com>
  • Loading branch information
dleavitt and alessbell committed May 3, 2023
1 parent 1b0a61f commit a625277
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 7 deletions.
9 changes: 9 additions & 0 deletions .changeset/warm-zebras-push.md
@@ -0,0 +1,9 @@
---
"@apollo/client": patch
---

Fix type signature of `ServerError`.

In <3.7 `HttpLink` and `BatchHttpLink` would return a `ServerError.message` of e.g. `"Unexpected token 'E', \"Error! Foo bar\" is not valid JSON"` and a `ServerError.result` of `undefined` in the case where a server returned a >= 300 response code with a response body containing a string that could not be parsed as JSON.

In >=3.7, `message` became e.g. `Response not successful: Received status code 302` and `result` became the string from the response body, however the type in `ServerError.result` was not updated to include the `string` type, which is now properly reflected.
20 changes: 19 additions & 1 deletion src/link/http/__tests__/HttpLink.ts
Expand Up @@ -1083,6 +1083,11 @@ describe('HttpLink', () => {
responseBody = JSON.parse(responseBodyText);
return Promise.resolve(responseBodyText);
});
const textWithStringError = jest.fn(() => {
const responseBodyText = 'Error! Foo bar';
responseBody = responseBodyText;
return Promise.resolve(responseBodyText);
});
const textWithData = jest.fn(() => {
responseBody = {
data: { stub: { id: 1 } },
Expand Down Expand Up @@ -1151,6 +1156,20 @@ describe('HttpLink', () => {
}),
);
});
itAsync('throws an error if response code is > 300 and handles string response body', (resolve, reject) => {
fetch.mockReturnValueOnce(Promise.resolve({ status: 302, text: textWithStringError }));
const link = createHttpLink({ uri: 'data', fetch: fetch as any });
execute(link, { query: sampleQuery }).subscribe(
result => {
reject('next should have been thrown from the network');
},
makeCallback(resolve, reject, (e: ServerError) => {
expect(e.message).toMatch(/Received status code 302/);
expect(e.statusCode).toBe(302);
expect(e.result).toEqual(responseBody);
}),
);
});
itAsync('throws an error if response code is > 300 and returns data', (resolve, reject) => {
fetch.mockReturnValueOnce(
Promise.resolve({ status: 400, text: textWithData }),
Expand Down Expand Up @@ -1180,7 +1199,6 @@ describe('HttpLink', () => {
);

const link = createHttpLink({ uri: 'data', fetch: fetch as any });

execute(link, { query: sampleQuery }).subscribe(
result => {
reject('should not have called result because we have no data');
Expand Down
2 changes: 1 addition & 1 deletion src/link/http/parseAndCheckHttpResponse.ts
Expand Up @@ -136,7 +136,7 @@ export function parseHeaders(headerText: string): Record<string, string> {
export function parseJsonBody<T>(response: Response, bodyText: string): T {
if (response.status >= 300) {
// Network error
const getResult = () => {
const getResult = (): Record<string, unknown> | string => {
try {
return JSON.parse(bodyText);
} catch (err) {
Expand Down
11 changes: 7 additions & 4 deletions src/link/persisted-queries/index.ts
Expand Up @@ -177,10 +177,13 @@ export const createPersistedQueryLink = (
}

// Network errors can return GraphQL errors on for example a 403
const networkErrors =
networkError &&
networkError.result &&
networkError.result.errors as GraphQLError[];
let networkErrors;
if (typeof networkError?.result !== 'string') {
networkErrors =
networkError &&
networkError.result &&
networkError.result.errors as GraphQLError[];
}
if (isNonEmptyArray(networkErrors)) {
graphQLErrors.push(...networkErrors);
}
Expand Down
2 changes: 1 addition & 1 deletion src/link/utils/throwServerError.ts
@@ -1,6 +1,6 @@
export type ServerError = Error & {
response: Response;
result: Record<string, any>;
result: Record<string, any> | string;
statusCode: number;
};

Expand Down

0 comments on commit a625277

Please sign in to comment.