Standard Error Functions

[paniavula]

Perceived Problem

"throw Error" from schema middleware return 500 http status code on the response.

Ideas / Proposed Solution(s)

This seems to be because of the issue graphql/express-graphql#427 in express-graphql server which is open from a very long time. Using the hijackresponse seem to be addressing it for now. Anything we can do in our framework for this?

Besides this, from a framework perspective it would be nice to standardise error functions like that done by Apollo in https://www.apollographql.com/docs/apollo-server/data/errors/. If these error functions can be matched by the respective response status codes it would be great.

[danielmahon]

FYI I am currently handling this by "hijacking" the express response and extending apollo-server-errors:

app.ts

server.express.use(formatErrors);

errors.ts

import { NextFunction, Request, Response } from 'express';
import hijackResponse from 'hijackresponse';
import { ApolloError } from 'apollo-server-errors';

export class LoginExpired extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'LOGIN_EXPIRED', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}
export class AuthenticationError extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'UNAUTHENTICATED', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}
export class ForbiddenError extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'FORBIDDEN', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}
export class UserInputError extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'BAD_USER_INPUT', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}

export class SyntaxError extends ApolloError {
  constructor(message: string) {
    super(message, 'GRAPHQL_PARSE_FAILED');

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}

export class ValidationError extends ApolloError {
  constructor(message: string) {
    super(message, 'GRAPHQL_VALIDATION_FAILED');

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}

/**
 * Stupid problems sometimes require stupid solutions.
 * Unfortunately `express-graphql` has hardcoded 4xx/5xx http status codes in certain error scenarios.
 * In addition they also finalize the response, so no other middleware shall prevail in their wake.
 *
 * It's best practice to always return 200 in GraphQL APIs and specify the error in the response,
 * as otherwise clients might choke on the response or unnecessarily retry stuff.
 * Also monitoring is improved by only throwing 5xx responses on unexpected server errors.
 *
 * This middleware will hijack the `res.send` method which gives us one last chance to modify
 * the response and normalize the response status codes.
 *
 * The only alternative to this would be to either fork or ditch `express-graphql`. ;-)
 */

// Extend Express Response with hijack specific function
interface IHijackedResponse extends Response {
  unhijack: () => void;
}

export const formatErrors = (
  _: Request,
  originalRes: Response,
  next: NextFunction,
) => {
  hijackResponse(originalRes, (err: Error, res: IHijackedResponse) => {
    // In case we encounter a "real" non GraphQL server error
    // we keep it untouched and move on.
    if (err) {
      res.unhijack();
      return next(err);
    }
    // We like our status code simple in GraphQL land
    // e.g. Apollo clients will retry on 5xx despite potentially not necessary.
    res.statusCode = 200;
    res.pipe(res);
  });
  // next() must be called explicitly, even when hijacking the response:
  next();
};

[nhuesmann]

I believe my issue is related to this. Previously, I was using a GraphQL Yoga server and the apollo-errors library to nicely format my errors. Example server init:

import { formatError } from 'apollo-errors';
import { GraphQLServer, Options } from 'graphql-yoga';

export const startServer = async () => {
  const server = new GraphQLServer();

  const options: Options = {
    formatError,
  };

  const app = await server.start(options);
  console.log(
    `GraphQL server is running at http://localhost:${port}${endpoint}`
  );

  return app;
};

Example from the apollo-errors repo of how errors are then formatted:

{
  "data": {},
  "errors": [
    {
      "message":"A foo error has occurred",
      "name":"FooError",
      "time_thrown":"2016-11-11T00:40:50.954Z",
      "data":{
        "something": "important"
      }
    }
  ]
}

With the latest version of Nexus, I can't figure out how to replicate this functionality. I tried using a middleware, formatting errors using the formatError function from the apollo-errors lib, then throwing the formatted error, however Nexus now errors with the following: nexus:server:graphql Unexpected error value.

I'd love Nexus to support GraphQL error formatting. Ideally, it would be as easy to configure as Apollo Server's formatError config option.

If anyone has any suggestions of a monkey patch way to do this for the time being, I'd love to hear them! @danielmahon based on the customization you've done with the hijackresponse lib, can you tell if using that lib would enable me to achieve this?

[Maetes]

FYI I am currently handling this by "hijacking" the express response and extending apollo-server-errors:

app.ts

server.express.use(formatErrors);

errors.ts

import { NextFunction, Request, Response } from 'express';
import hijackResponse from 'hijackresponse';
import { ApolloError } from 'apollo-server-errors';

export class LoginExpired extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'LOGIN_EXPIRED', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}
export class AuthenticationError extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'UNAUTHENTICATED', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}
export class ForbiddenError extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'FORBIDDEN', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}
export class UserInputError extends ApolloError {
  constructor(message: string, properties?: Record<string, any>) {
    super(message, 'BAD_USER_INPUT', properties);

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}

export class SyntaxError extends ApolloError {
  constructor(message: string) {
    super(message, 'GRAPHQL_PARSE_FAILED');

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}

export class ValidationError extends ApolloError {
  constructor(message: string) {
    super(message, 'GRAPHQL_VALIDATION_FAILED');

    Object.defineProperty(this, 'name', { value: this.constructor.name });
  }
}

/**
 * Stupid problems sometimes require stupid solutions.
 * Unfortunately `express-graphql` has hardcoded 4xx/5xx http status codes in certain error scenarios.
 * In addition they also finalize the response, so no other middleware shall prevail in their wake.
 *
 * It's best practice to always return 200 in GraphQL APIs and specify the error in the response,
 * as otherwise clients might choke on the response or unnecessarily retry stuff.
 * Also monitoring is improved by only throwing 5xx responses on unexpected server errors.
 *
 * This middleware will hijack the `res.send` method which gives us one last chance to modify
 * the response and normalize the response status codes.
 *
 * The only alternative to this would be to either fork or ditch `express-graphql`. ;-)
 */

// Extend Express Response with hijack specific function
interface IHijackedResponse extends Response {
  unhijack: () => void;
}

export const formatErrors = (
  _: Request,
  originalRes: Response,
  next: NextFunction,
) => {
  hijackResponse(originalRes, (err: Error, res: IHijackedResponse) => {
    // In case we encounter a "real" non GraphQL server error
    // we keep it untouched and move on.
    if (err) {
      res.unhijack();
      return next(err);
    }
    // We like our status code simple in GraphQL land
    // e.g. Apollo clients will retry on 5xx despite potentially not necessary.
    res.statusCode = 200;
    res.pipe(res);
  });
  // next() must be called explicitly, even when hijacking the response:
  next();
};

I love your Answer and it kills this ugly stucking 500er errors. Thank you very much! But still i got some behaviour which i want to check with you whether this is usual:

When i go to chrome dev tools the 500 sstuck is gone but still chrome blocks on this:

EDIT: I'm able to clear this break by inserting. the errorPolicy: 'all' to the hook but have no clue why it's not thrown in chrome then anymore...

is this usual for apollo Client to block chrome when dev tools is open or can i avoid this?

Thanks

[mohe2015]

I THINK this is fixed as apollo is now used instead of express.

[jasonkuhrt]

@mohe2015 yep thanks and Thanks @izziaraffaele