Proper error handling [minor changes required]: fixing Object object error messages

[ghost]

@hwillson for quite a while me and other GraphQL users were dealing with unclear error messages.

To name a few:

dotansimha/graphql-binding#173
#743
(and maybe, but I'm not sure) #1037

The impossibility of using custom error codes that Apollo offers and dealing with weird error messages makes it hard to handle errors in a complex application.

Long story short, I guess I've managed to find the possible cause (at line 110):

https://github.com/apollographql/graphql-tools/blob/2394f0c3a8b30dbd4435dd21019727e76e61b892/src/stitching/errors.ts#L104-L113

If the checkResultAndHandleErrors is called and there's an errors array contains only one error the resulting error will be an instance not of Error but rather Object, which will result in unexpected behaviour here - https://github.com/graphql/graphql-js/blob/c2bc19badebd208ff21849e8f579e7034631af42/src/execution/execute.js#L721 as a result of this logic https://github.com/graphql/graphql-js/blob/c2bc19badebd208ff21849e8f579e7034631af42/src/execution/execute.js#L753-L758

In my opinion, the simplest solution would be to have something like this:

  if (result.errors && (!result.data || result.data[responseKey] == null)) {
    // apollo-link-http & http-link-dataloader need the
    // result property to be passed through for better error handling.
    // If there is only one error, which contains a result property, pass the error through
    const newError =
      result.errors.length === 1 && hasResult(result.errors[0])
        // Setting the prototype of result.errors[0] to Error
        ? Object.setPrototypeOf(result.errors[0], Error.prototype)
        : new CombinedError(concatErrors(result.errors), result.errors);
    throw locatedError(newError, info.fieldNodes, responsePathAsArray(info.path));
 } 

[terion-name]

Very needed fix!

[marjandjokic]

Yes, this is really needed fix, as I have the same problem and proposed fix solves the problem

[fluggo]

Fix is perfect. Until it lands, I'm using the following equivalent workaround for our remote schemas:

this.link = new ApolloLink((operation: Operation, forward?: NextLink) => {
  return forward(operation).map(data => {
    if(data.errors) {
      for(const error of data.errors) {
        if(!(error instanceof Error))
            Object.setPrototypeOf(error, Error.prototype);
      }
    }

    return data;
  });
}).concat(link);

[tsbhangu]

Agreed! The errors keep getting swallowed and it's very annoying to go to the base layer application to try to figure out what the error is.

[robertontiu]

Any progress with this issue?

[kamilkisiela]

We recently released an alpha version of GraphQL Tools (#1308) that should fix the issue.

Please update graphql-tools to next or run:

npx match-version graphql-tools next

Let us know if it solves the problem, we're counting for your feedback! :)

[yaacovCR]

Folded into #1306