Skip to content
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.

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

Fix type signature of ServerError #10810

Merged
merged 7 commits into from
May 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 9 additions & 0 deletions .changeset/warm-zebras-push.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
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
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export type ServerError = Error & {
response: Response;
result: Record<string, any>;
result: Record<string, any> | string;
statusCode: number;
};

Expand Down