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 persistedQueries to pull graphqlErrors from networkErrors #9410

Merged
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
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
## Apollo Client 3.6.2 (unreleased)

- Consider `networkError.result.errors` in addition to `result.errors` in `PersistedQueryLink`. <br/>
[@redaid113](https://github.com/redaid113) and [@benjamn](https://github.com/benjamn) in [#9410](https://github.com/apollographql/apollo-client/pull/9410)

## Apollo Client 3.6.1 (2022-04-28)

### Bug Fixes
Expand Down
29 changes: 29 additions & 0 deletions src/link/persisted-queries/__tests__/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -489,4 +489,33 @@ describe('failure path', () => {
},
);
});

itAsync('handles 400 response network error and graphql error without disabling persistedQuery support', (resolve, reject) => {
let failed = false;
fetch.mockResponseOnce(response);

const fetcher = (...args: any[]) => {
if (!failed) {
failed = true;
return Promise.resolve({
json: () => Promise.resolve(errorResponse),
text: () => Promise.resolve(errorResponse),
status: 400,
});
}
return fetch(...args);
};

const link = createPersistedQuery({ sha256 }).concat(
createHttpLink({ fetch: fetcher } as any),
);

execute(link, { query, variables }).subscribe(result => {
expect(result.data).toEqual(data);
const [, success] = fetch.mock.calls[0];
expect(JSON.parse(success!.body!.toString()).query).toBe(queryString);
expect(JSON.parse(success!.body!.toString()).extensions).not.toBeUndefined();
resolve();
}, reject);
});
});
57 changes: 41 additions & 16 deletions src/link/persisted-queries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,16 @@ import {
Observer,
ObservableSubscription,
compact,
isNonEmptyArray,
} from '../../utilities';
import { NetworkError } from '../../errors';
import { ServerError } from '../utils';

export const VERSION = 1;

export interface ErrorResponse {
graphQLErrors?: readonly GraphQLError[];
networkError?: Error;
networkError?: NetworkError;
response?: ExecutionResult;
operation: Operation;
}
Expand All @@ -46,18 +49,29 @@ export namespace PersistedQueryLink {
export type Options = SHA256Options | GenerateHashOptions;
}

function collectErrorsByMessage<TError extends Error>(
graphQLErrors: TError[] | readonly TError[] | undefined,
): Record<string, TError> {
const collected: Record<string, TError> = Object.create(null);
if (isNonEmptyArray(graphQLErrors)) {
graphQLErrors.forEach(error => collected[error.message] = error);
}
return collected;
}

const defaultOptions = {
disable: ({ graphQLErrors, operation }: ErrorResponse) => {
const errorMessages = collectErrorsByMessage(graphQLErrors);

// if the server doesn't support persisted queries, don't try anymore
if (
graphQLErrors &&
graphQLErrors.some(
({ message }) => message === 'PersistedQueryNotSupported',
)
) {
if (errorMessages.PersistedQueryNotSupported) {
return true;
}

if (errorMessages.PersistedQueryNotFound) {
return false;
}

const { response } = operation.getContext();
// if the server responds with bad request
// apollo-server responds with 400 for GET and 500 for POST when no query is found
Expand Down Expand Up @@ -158,30 +172,41 @@ export const createPersistedQueryLink = (
{
response,
networkError,
}: { response?: ExecutionResult; networkError?: Error },
}: { response?: ExecutionResult; networkError?: ServerError },
cb: () => void,
) => {
if (!retried && ((response && response.errors) || networkError)) {
retried = true;

const graphQLErrors: GraphQLError[] = [];

const responseErrors = response && response.errors;
if (isNonEmptyArray(responseErrors)) {
graphQLErrors.push(...responseErrors);
}

// Network errors can return GraphQL errors on for example a 403
const networkErrors =
networkError &&
networkError.result &&
networkError.result.errors as GraphQLError[];
if (isNonEmptyArray(networkErrors)) {
graphQLErrors.push(...networkErrors);
}

const disablePayload = {
response,
networkError,
operation,
graphQLErrors: response ? response.errors : undefined,
graphQLErrors: isNonEmptyArray(graphQLErrors) ? graphQLErrors : void 0,
};

// if the server doesn't support persisted queries, don't try anymore
supportsPersistedQueries = !disable(disablePayload);

// if its not found, we can try it again, otherwise just report the error
if (
(response &&
response.errors &&
response.errors.some(
({ message }: { message: string }) =>
message === 'PersistedQueryNotFound',
)) ||
collectErrorsByMessage(graphQLErrors).PersistedQueryNotFound ||
!supportsPersistedQueries
) {
// need to recall the link chain
Expand Down Expand Up @@ -213,7 +238,7 @@ export const createPersistedQueryLink = (
next: (response: ExecutionResult) => {
retry({ response }, () => observer.next!(response));
},
error: (networkError: Error) => {
error: (networkError: ServerError) => {
retry({ networkError }, () => observer.error!(networkError));
},
complete: observer.complete!.bind(observer),
Expand Down