Handling non Error instance errors is broken

[geogeim]

Currently graphql handles resolve error as follows:

function asErrorInstance(error) {
  return error instanceof Error ? error : new Error(error || undefined);
}

This is problematic because if the error is not a string or an instance of Error then the error will be serialized as [object Object] and the workaround (filling up the code with try-catch statements and converting them to Error instances) defeats the purpose of handling errors at the graphql library level.

Please provide a configurable mechanism for dealing with errors.

[IvanGoncharov]

@geogeim Can you please describe your use case in more details?
What kind of error object is throw from resolver?
Why it can't be a subclass of Error?

[geogeim]

@IvanGoncharov it's a plain object thrown from a third party npm module (twillio, authy)

[IvanGoncharov]

@geogeim This feature was implemented in #1600 and scheduled to for 14.2.0 release.

[marhub]

In my opinion feature implemented in #1600 is not enough for some more complex use cases.

I have gateway that passes all traffic to sub-services also written in graphql. When subservice returns any error, i want to pass that error directly to client. Currently error is returned in format:

  "errors": [
    {
      "message": "Unexpected error value: <JSON_FROM_SUB_SERVICE_HERE>"
}]

so to get details like original message, extensions etc - i have to remove "Unexpected error value:" from string and JSON.parse contents - that's hackish.

I think better solution would be to join original error into new Error instance:

  let returnError = new Error('Unexpected error value: ' + (0, _inspect.default)(error));
  returnError.originalError = error;
  return returnError;

@IvanGoncharov what do you think ?

[IvanGoncharov]

I have gateway that passes all traffic to sub-services also written in graphql.

@marhub You should check if server response return errors and construct new GraphQLError yourself.

In my opinion feature implemented in #1600 is not enough for some more complex use cases.

It was never intended to be a solution for the problem it's just a better error message to help you debug this issue and correctly fix it by constructing new GraphQLError yourself.

In JS we should throw only instances of Error, more details here:
https://eslint.org/docs/rules/no-throw-literal

[marhub]

You should check if server response return errors and construct new GraphQLError yourself.

i'm using prisma/graphql-yoga that wraps around apollo server - I could not find any way to do so...
The chain of dependencies is so long, I find it hard to understand how errors are constructed, and my PR seems to be best solution.

edit: please note: gateway = apollo server, it connects to few services ( remote schema ), errors come from those remote services.

[IvanGoncharov]

The chain of dependencies is so long, I find it hard to understand how errors are constructed, and my PR seems to be best solution.

@marhub graphlql-yoga and apollo server use graphql-js not the other way around.
We shouldn't add workarounds for the bugs inside those projects.

If we all agree that you should always throw instances of Error than there is nothing we can do in this project beyond #1600

[IvanGoncharov]

@marhub I understand that you want to solve the issue with your particular server.
So from the practical point, you can either fork this repo or use something like https://github.com/ds300/patch-package

In long run, you should report this issue to graphql-yoga and apollo-server so they could fix it.

[marhub]

@IvanGoncharov done some digging and found actual error in apollo/graphql-tools.
Thanks for your help!