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

Change usages of the GraphQLError type to GraphQLFormattedError. #11789

Merged
merged 12 commits into from
Jul 9, 2024
5 changes: 5 additions & 0 deletions .changeset/slimy-berries-yawn.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@apollo/client": patch
---

post-process errors received from responses into GraphQLError instances
4 changes: 2 additions & 2 deletions src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { DocumentNode, GraphQLError } from "graphql";
import type { DocumentNode, GraphQLFormattedError } from "graphql";
import { equal } from "@wry/equality";

import type { Cache, ApolloCache } from "../cache/index.js";
Expand Down Expand Up @@ -82,7 +82,7 @@ export class QueryInfo {
variables?: Record<string, any>;
networkStatus?: NetworkStatus;
networkError?: Error | null;
graphQLErrors?: ReadonlyArray<GraphQLError>;
graphQLErrors?: ReadonlyArray<GraphQLFormattedError>;
stopped = false;

private cache: ApolloCache<any>;
Expand Down
3 changes: 2 additions & 1 deletion src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import {
ApolloError,
isApolloError,
graphQLResultHasProtocolErrors,
reviveGraphQLError,
} from "../errors/index.js";
import type {
QueryOptions,
Expand Down Expand Up @@ -1210,7 +1211,7 @@ export class QueryManager<TStore> {
};

if (hasErrors && options.errorPolicy !== "ignore") {
aqr.errors = graphQLErrors;
aqr.errors = graphQLErrors.map(reviveGraphQLError);
aqr.networkStatus = NetworkStatus.error;
}

Expand Down
36 changes: 35 additions & 1 deletion src/errors/__tests__/ApolloError.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { ApolloError } from "..";
import { GraphQLError } from "graphql";
import { ExecutableDefinitionNode, GraphQLError, parse, Source } from "graphql";

describe("ApolloError", () => {
it("should construct itself correctly", () => {
Expand Down Expand Up @@ -107,4 +107,38 @@ describe("ApolloError", () => {
});
expect(apolloError.stack).toBeDefined();
});

it("will revive `GraphQLError` instances from `graphQLErrors`", () => {
const source = new Source(`
{
field
}
`);
const ast = parse(source);
const operationNode = ast.definitions[0] as ExecutableDefinitionNode;
const fieldNode = operationNode.selectionSet.selections[0];
const original = new GraphQLError("msg" /* message */, {
nodes: [fieldNode],
source,
positions: [1, 2, 3],
path: ["a", "b", "c"],
originalError: new Error("test"),
extensions: { foo: "bar" },
});

const apolloError = new ApolloError({
graphQLErrors: [JSON.parse(JSON.stringify(original))],
});
const graphQLError = apolloError.graphQLErrors[0];

console.log({
graphQLError,
original,
serialized: JSON.stringify(original),
});

expect(graphQLError).toBeInstanceOf(GraphQLError);
// test equality of enumberable fields. non-enumerable fields will differ
expect({ ...graphQLError }).toStrictEqual({ ...original });
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After writing this test I'm less convinced that this is the path forward - it's not straightforward to deserialize a serialized GraphQLError as the serialized variant is missing a lot of information and the constructor doesn't even accept locations.

});
});
33 changes: 28 additions & 5 deletions src/errors/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import "../utilities/globals/index.js";

import type { GraphQLError, GraphQLErrorExtensions } from "graphql";
import type { GraphQLErrorExtensions, GraphQLFormattedError } from "graphql";
import { GraphQLError } from "graphql";

import { isNonNullObject } from "../utilities/index.js";
import type { ServerParseError } from "../link/http/index.js";
Expand All @@ -17,7 +18,7 @@ type FetchResultWithSymbolExtensions<T> = FetchResult<T> & {
};

export interface ApolloErrorOptions {
graphQLErrors?: ReadonlyArray<GraphQLError>;
graphQLErrors?: ReadonlyArray<GraphQLFormattedError>;
protocolErrors?: ReadonlyArray<{
message: string;
extensions?: GraphQLErrorExtensions[];
Expand Down Expand Up @@ -74,7 +75,7 @@ export type NetworkError = Error | ServerParseError | ServerError | null;
export class ApolloError extends Error {
public name: string;
public message: string;
public graphQLErrors: GraphQLErrors;
public graphQLErrors: GraphQLError[];
public protocolErrors: ReadonlyArray<{
message: string;
extensions?: GraphQLErrorExtensions[];
Expand Down Expand Up @@ -111,7 +112,7 @@ export class ApolloError extends Error {
}: ApolloErrorOptions) {
super(errorMessage);
this.name = "ApolloError";
this.graphQLErrors = graphQLErrors || [];
this.graphQLErrors = (graphQLErrors || []).map(reviveGraphQLError);
this.protocolErrors = protocolErrors || [];
this.clientErrors = clientErrors || [];
this.networkError = networkError || null;
Expand All @@ -120,7 +121,7 @@ export class ApolloError extends Error {
this.cause =
[
networkError,
...(graphQLErrors || []),
...(this.graphQLErrors || []),
...(protocolErrors || []),
...(clientErrors || []),
].find((e) => !!e) || null;
Expand All @@ -130,3 +131,25 @@ export class ApolloError extends Error {
(this as any).__proto__ = ApolloError.prototype;
}
}

/**
* Revives a GraphQL error that has been received over the network.
* Some fields of GraphQL errors, e.g. `extensions`, are not mandatory in the
* spec, but they are not optional in the `GraphQLError` type.
* This function ensures that all errors are instances of the `GraphQLError` class.
*/
export function reviveGraphQLError(error: GraphQLFormattedError): GraphQLError {
return error instanceof GraphQLError ? error : (
Object.assign(
new GraphQLError(
error.message,
// This will pass `message` into the `options` parameter.
// The constructor does not expect that, but it will ignore it and we
// don't need to destructure `error`, saving some bundle size.
error
),
//
{ locations: error.locations }
)
);
}
6 changes: 4 additions & 2 deletions src/link/core/types.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import type { ExecutionResult, GraphQLError } from "graphql";
import type { GraphQLError, GraphQLFormattedError } from "graphql";
import type { DocumentNode } from "graphql";
import type { DefaultContext } from "../../core/index.js";
export type { DocumentNode };
Expand Down Expand Up @@ -91,10 +91,12 @@ export interface SingleExecutionResult<
TData = Record<string, any>,
TContext = DefaultContext,
TExtensions = Record<string, any>,
> extends ExecutionResult<TData, TExtensions> {
> {
// data might be undefined if errorPolicy was set to 'ignore'
data?: TData | null;
context?: TContext;
errors?: ReadonlyArray<GraphQLFormattedError>;
extensions?: TExtensions;
}

export type FetchResult<
Expand Down
6 changes: 3 additions & 3 deletions src/link/error/index.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
import type { ExecutionResult } from "graphql";
import type { ExecutionResult, GraphQLFormattedError } from "graphql";

import type { NetworkError, GraphQLErrors } from "../../errors/index.js";
import type { NetworkError } from "../../errors/index.js";
import { Observable } from "../../utilities/index.js";
import type { Operation, FetchResult, NextLink } from "../core/index.js";
import { ApolloLink } from "../core/index.js";

export interface ErrorResponse {
graphQLErrors?: GraphQLErrors;
graphQLErrors?: ReadonlyArray<GraphQLFormattedError>;
networkError?: NetworkError;
response?: ExecutionResult;
operation: Operation;
Expand Down Expand Up @@ -38,7 +38,7 @@
if (result.errors) {
retriedResult = errorHandler({
graphQLErrors: result.errors,
response: result,

Check failure on line 41 in src/link/error/index.ts

View workflow job for this annotation

GitHub Actions / cleanup

Type 'ExecutionPatchInitialResult<Record<string, any>, Record<string, any>> | SingleExecutionResult<Record<string, any>, Record<string, any>, Record<...>>' is not assignable to type 'ExecutionResult<ObjMap<unknown>, ObjMap<unknown>> | undefined'.

Check failure on line 41 in src/link/error/index.ts

View workflow job for this annotation

GitHub Actions / size

Type 'ExecutionPatchInitialResult<Record<string, any>, Record<string, any>> | SingleExecutionResult<Record<string, any>, Record<string, any>, Record<...>>' is not assignable to type 'ExecutionResult<ObjMap<unknown>, ObjMap<unknown>> | undefined'.

Check failure on line 41 in src/link/error/index.ts

View workflow job for this annotation

GitHub Actions / Compare Build Output

Type 'ExecutionPatchInitialResult<Record<string, any>, Record<string, any>> | SingleExecutionResult<Record<string, any>, Record<string, any>, Record<...>>' is not assignable to type 'ExecutionResult<ObjMap<unknown>, ObjMap<unknown>> | undefined'.

Check failure on line 41 in src/link/error/index.ts

View workflow job for this annotation

GitHub Actions / Api-Extractor

Type 'ExecutionPatchInitialResult<Record<string, any>, Record<string, any>> | SingleExecutionResult<Record<string, any>, Record<string, any>, Record<...>>' is not assignable to type 'ExecutionResult<ObjMap<unknown>, ObjMap<unknown>> | undefined'.

Check failure on line 41 in src/link/error/index.ts

View workflow job for this annotation

GitHub Actions / Are the types wrong

Type 'ExecutionPatchInitialResult<Record<string, any>, Record<string, any>> | SingleExecutionResult<Record<string, any>, Record<string, any>, Record<...>>' is not assignable to type 'ExecutionResult<ObjMap<unknown>, ObjMap<unknown>> | undefined'.

Check failure on line 41 in src/link/error/index.ts

View workflow job for this annotation

GitHub Actions / cleanup

Type 'ExecutionPatchInitialResult<Record<string, any>, Record<string, any>> | SingleExecutionResult<Record<string, any>, Record<string, any>, Record<...>>' is not assignable to type 'ExecutionResult<ObjMap<unknown>, ObjMap<unknown>> | undefined'.
operation,
forward,
});
Expand Down
16 changes: 12 additions & 4 deletions src/link/persisted-queries/index.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { invariant } from "../../utilities/globals/index.js";

import { print } from "../../utilities/index.js";
import type { DocumentNode, ExecutionResult, GraphQLError } from "graphql";
import type {
DocumentNode,
ExecutionResult,
GraphQLError,
GraphQLFormattedError,
} from "graphql";

import type { Operation } from "../core/index.js";
import { ApolloLink } from "../core/index.js";
Expand All @@ -21,7 +26,7 @@ import {
export const VERSION = 1;

export interface ErrorResponse {
graphQLErrors?: readonly GraphQLError[];
graphQLErrors?: ReadonlyArray<GraphQLFormattedError>;
networkError?: NetworkError;
response?: ExecutionResult;
operation: Operation;
Expand Down Expand Up @@ -59,7 +64,10 @@ export namespace PersistedQueryLink {
}

function processErrors(
graphQLErrors: GraphQLError[] | readonly GraphQLError[] | undefined
graphQLErrors:
| GraphQLFormattedError[]
| ReadonlyArray<GraphQLFormattedError>
| undefined
): ErrorMeta {
const byMessage = Object.create(null),
byCode = Object.create(null);
Expand Down Expand Up @@ -180,7 +188,7 @@ export const createPersistedQueryLink = (
if (!retried && ((response && response.errors) || networkError)) {
retried = true;

const graphQLErrors: GraphQLError[] = [];
const graphQLErrors: GraphQLFormattedError[] = [];

const responseErrors = response && response.errors;
if (isNonEmptyArray(responseErrors)) {
Expand Down
Loading