Improved error handling in GraphQL APIs

[michaelbromley]

Is your feature request related to a problem? Please describe.
Current error handling is pretty basic: throw an error from anywhere in the stack. This results in the error populating the GraphQL errors array in the response.

There are a few problems with this:

1. There is no way to know what errors can be returned from a particular operation
2. Errors in a nested resolver will nuke the entire request, even when the parent data is fine.
3. It is difficult for clients to handle errors in a smart way - the client is stuck having to check the errors array which is inconvenient
4. When performing an operation like createAssets, if uploading 5 images and 1 fails, there's no way to return 4 good results and 1 error result.

Describe the solution you'd like
This talk describes a good solution that solves all of the above: https://www.youtube.com/watch?v=RDNTP66oY2o

The basic idea is to define results as a union of happy-path and error path results, so that all the "expected" errors are encoded into the schema and can explicitly queried for and handled by the client.

Example: the createAssets mutation:

# before
type Mutation {
    createAssets(input: [CreateAssetInput!]!): [Asset!]!
}

# after
interface ErrorResult {
    message: String!
}

type InvalidMimeType implements ErrorResult {
    message: String!
}

union CreateAssetResult = Asset | InvalidMimeType

type Mutation {
    createAssets(input: [CreateAssetInput!]!): [CreateAssetResult!]!
}

This would then be executed on the client with a mutation like:

    mutation CreateAssets($input: [CreateAssetInput!]!) {
        createAssets(input: $input) {
            ... on Asset {
               id
               preview
               # ... etc
            }
            ... on InvalidMimeType {
              message
            }
        }
    }

Challenges
This would require some quite significant re-work of the schema, as a new result union will need to be defined for the majority of operations. Also to keep the resolvers type safe, we'd want to always use the generated return types, rather than the entities as is currently the case for most of the resolvers. This part would depend on #410 being solved I think.

[michaelbromley]

I got 100% positive feedback on this proposal on the Vendure slack & via Twitter. I'll start a proof-of-concept implementation now.

A bit more research follows:

Further reading

• 200 OK! Error Handling in GraphQL The blog post from Sasha Solomon that accompanies the video linked above.
• Where art thou, my error? A detailed look at the same idea
• Handling GraphQL errors like a champ with unions and interfaces Same again

Some key points:

Error types

You can think of 2 "types" of errors:

1. Errors where something unexpected went wrong, e.g. internal server errors
2. Errors that are entirely possible and expected, e.g. custom field validation error when mutating an entity.

The first kind we can continue to handle in the GraphQL "errors" array. There's no logical relationship between a particular mutation and some random InternalServerError, so it makes no sense to encode this in the schema.

The second kind is the one we are interested in encoding into the schema.

Handling in the client

Once we encode all expected response types into the schema, the client can then handle each appropriately using the fragment syntax. Since we will use a common interface as the basis of all errors, clients can always select this interface to ensure the error is caught, even if the specific implementation types change over time.

Alternative solution: error fields in result

The Apollo docs recommend adding extra fields to mutation responses:

To help resolve both of these concerns, we recommend defining a MutationResponse interface in your schema, along with a collection of object types that implement that interface (one for each of your mutations).

interface MutationResponse {
 code: String!
 success: Boolean!
 message: String!
}

Here's what the MutationResponse interface looks like:

type UpdateUserEmailMutationResponse implements MutationResponse {
 code: String!
 success: Boolean!
 message: String!
 user: User
}

The Python GraphQL client Ariadne recommends something similar

type_def = """
    type Mutation {
        login(username: String!, password: String!) {
            error: String
            user: User
        }
    }
"""

A criticism of this approach is given in the "Where art thou?" blog post:

Adding these fields to the same namespace makes sense when we’re thinking of the failure case, but what about the success case? Do we really need a success boolean to indicate that updates to the artwork were made? What purpose serves the message field, other than possibly being a sign of an overly positive schema that sends you happy messages?

Another downside of this approach is that we don't get the same level of type-safety. With the union types, we can do things like exhaustiveness checks to make sure we always handle all possible result types, and we get static typing on particular properties that may be defined on particular error results.

[michaelbromley]

Proof of concept

I've put together a proof-of-concept for the createAssets mutation. Here are the parts:

ErrorResult interface

This is the interface implemented by all error results:

enum ErrorCode {
    UnknownError
}

interface ErrorResult {
    code: ErrorCode!
    message: String!
}

Concrete error result
This is the definition for the createAssets mutation:

type Mutation {
    "Create a new Asset"
    createAssets(input: [CreateAssetInput!]!): [CreateAssetResult!]!
    # ...
}

type MimeTypeError implements ErrorResult {
    code: ErrorCode!
    message: String!
    fileName: String!
    mimeType: String!
}

union CreateAssetResult = Asset | MimeTypeError

Note that the ErrorCode enum is automatically populated at run-time, with the value being equal to the type name, so with the above code, the run-time value of the enum is:

enum ErrorCode {
  UnknownError
  MimeTypeError
}

Code generation
In the AssetService.createAssetInternal() method, we need to return the MimeTypeError if the mime type of the file is not valid. To make this slightly more convenient for the developer, I've written a plugin for the graphql-code-generator package (which we already use extensively to create TS typings for our GraphQL types/operations). This plugin does 3 things:

• Generate classes for the error results
• Generate a type guard utility function to determine whether an object is of the ErrorResult type. This is useful for correctly handling the results in TS code, keeping the correct type information flowing.
• Generate a type resolver for Apollo Server for each of the union types.

The generated code looks like this:

// tslint:disable
/** This file was generated by the graphql-errors-plugin, which is part of the "codegen" npm script. */

import { ErrorCode } from '@vendure/common/lib/generated-types';

export type Scalars = {
    ID: string;
    String: string;
    Boolean: boolean;
    Int: number;
    Float: number;
    DateTime: any;
    JSON: any;
    Upload: any;
};

export class ErrorResult {
    readonly __typename: string;
    readonly code: ErrorCode;
    message: Scalars['String'];
}

export class MimeTypeError extends ErrorResult {
    readonly __typename = 'MimeTypeError';
    readonly code = ErrorCode.MimeTypeError;
    constructor(
        public message: Scalars['String'],
        public fileName: Scalars['String'],
        public mimeType: Scalars['String'],
    ) {
        super();
    }
}

const errorTypeNames = new Set(['MimeTypeError']);
export function isGraphQLError(
    input: any,
): input is import('@vendure/common/lib/generated-types').ErrorResult {
    return input instanceof ErrorResult || errorTypeNames.has(input.__typename);
}

export const adminErrorOperationTypeResolvers = {
    CreateAssetResult: {
        __resolveType(value: any) {
            return isGraphQLError(value) ? (value as any).__typename : 'Asset';
        },
    },
};

Returning error results from services
The AssetService code now looks like this:

private async createAssetInternal(
  ctx: RequestContext,
  stream: Stream,
  filename: string,
  mimetype: string,
  ): Promise<CreateAssetResult> {
    const { assetOptions } = this.configService;
    if (!this.validateMimeType(mimetype)) {
      //  throw new UserInputError('error.mime-type-not-permitted', { mimetype }); 
      return new MimeTypeError('error.mime-type-not-permitted', filename, mimetype);
    }
    // ... create Asset
}

And we can correctly discriminate between the actual Asset and any error results using the generated isErrorResult() function:

/**
 * Create an Asset based on a file uploaded via the GraphQL API.
 */
async create(ctx: RequestContext, input: CreateAssetInput): Promise<CreateAssetResult> {
  const { createReadStream, filename, mimetype } = await input.file;
  const stream = createReadStream();
  const result = await this.createAssetInternal(ctx, stream, filename, mimetype);
  if (isGraphQLError(result)) {
    return result;
  }
  this.eventBus.publish(new AssetEvent(ctx, result, 'created'));
  return result;
}

Next steps

Next I will go through all points where errors are thrown, and determine whether they should stay as thrown errors (e.g. InternalServerError) or whether they should be returned as an ErrorResult with the resulting change to the return type of the GraphQL operation (e.g. with UserInputError).

An open question is how to handle translations of error messages. I am debating whether or not it makes sense to even do i18n on server error messages. It might make more sense to just return a code and let it be translated on the client. Not decided yet so I'm going to leave this to the end.

[mschipperheyn]

I have been following this approach. The problem for me so far is that the resolver throws an "Unexpected error value" with the custom error as key that leads to the graphql query throwing instead of returning the error as the union result

Serialized Error: { response: { errors: [ { message: 'Unexpected error value: { message: "VALIDATION_ERROR", errors: [[Object]], fieldErrors: { startDate: "Should be after end date" } }', locations: [ { line: 2, column: 3 } ], path: [ 'rsv_addLocationBlackoutDate' ] } ], data: null, status: 200 }, request: { query: 'mutation rsv_addLocationBlackoutDate($input: RsvBlackoutDateInput!, $locationId: ID!) {\n  rsv_addLocationBlackoutDate(input: $input, locationId: $locationId) {\n    ... on RsvBlackoutDate {\n      id\n      startDate\n      endDate\n    }\n    ... on ErrorResult {\n      errorCode\n      message\n    }\n  }\n}', variables: { input: { startDate: '2024-07-07', endDate: '2024-08-07' }, locationId: 'T_3' } } }