GraphQL Error Swallowed Natively but works in Express-graphql

[alifaff]

Using Graphql 0.13.2

Executing graphql natively as follows through AWS Lambda:

index.js - calls

const runGraphQL = (query, variables, params) => {
  return graphql(schema, query, null, {params}, variables)
}

If the executing code throws an error, it usually returns message as string with [object, Object]. I suspect it wraps to GraphQLError. If I wrap the above error object using GraphQLError then I do not get anything back and my response.errors is an empty object {}. The only way I am able to get back data properly is to return an Error as a string or a GraphQLError as a string, I am unable to push back an object.

Oddly enough if I run GraphQL with express and add the formatError with an error handler then that works, however using it natively it does not.

It seems that I am missing something simple to get GraphQL to return the object instead of only working with strings. Any ideas?

[IvanGoncharov]

return graphql(schema, query, null, {params}, variables)

@alifaff You shouldn't wrap params into the object.

It seems that I am missing something simple to get GraphQL to return the object instead of only working with strings. Any ideas?

Not sure that I fully understand your question. Can you please provide a minimal example of the actual and expected output.
Also, do you use JSON.stringify to generate response string?

[alifaff]

Sorry...the params was meant as a place holder for loaders and other context items (should have used different terminology).

See below a sample mutation:

const createClient = {
  type: ClientCreateResponseType,
  args: clientCreateArgs,
  resolve: (root, args, {event, db}) => {
    // Validates arguments and returns custom error objects of type ApiError (see below for code)
    schemaValidator.validateSchema(clientCreateArgs, args)
    return clientHandler.saveClient(db, args, empId)
  }
}
module.exports.createClient = createClient

class ApiError extends Error {
  constructor(type, ...errors) {
    super(...errors)
    if (Error.captureStackTrace) {
      Error.captureStackTrace(this, ApiError)
    }
    this.date = new Date()
    this.type = type
    this.errors = errors
  }
}
module.exports.ApiError = ApiError

The function createClient generates a custom error as below:

{
   date: "2018-04-25T15:22:21",
   type: "INPUT",
   errors: [
     { key: 'firstName', message: 'should NOT be shorter than 2 characters' },
     { key: 'lastName', message: 'should NOT be shorter than 2 characters' }
   ]
}

We would like the output to be as follows:

{
  "errors": [
    {
      "type": "INPUT",
      "date": "2018-04-25T15:22:21",
      "errors": [
         { key: 'firstName', message: 'should NOT be shorter than 2 characters' },
         { key: 'lastName', message: 'should NOT be shorter than 2 characters' }
       ]
    }
  ],
  "data": {
    "createClient": null
  }
}

The above works fine with express-graphql however using it through Lambda and natively calling graphql graphql(schema, query, null, {contextParams}, variables) does not seem to return an error rather only get back the following:

{
    "message": "[object Object]",
    "locations": [
        {
            "line": 1,
            "column": 24
        }
    ],
    "path": [
        "createClient"
    ]
}

The setup is the same for both using the same schema, however the only difference is that it does not contain the formatError. Below is the code when using express-graphql:

const express = require('express')
const graphqlHTTP = require('express-graphql')
const schema = require('../app/graphql/schema').schema
const loaders = require('../app/graphql/loaders').loaders

const app = express()
app.use((req, res, next) => {
  res.header('Access-Control-Allow-Credentials', 'true')
  res.header('Access-Control-Allow-Origin', 'http://localhost:8080')
  res.header('Access-Control-Allow-Methods', 'GET, PUT, POST')
  res.header('Access-Control-Allow-Headers', 'Origin, X-Requested-With, Content-Type, Accept, AT-Request-Id')
  res.header('')
  next()
})
app.use(express.static(__dirname))
app.use('/graphql', graphqlHTTP(() => (
    {
      context: {loaders, event},
      schema,
      graphiql: true,
      formatError: graphqlErrorManager.manageGraphErrors // Custom graphqlError wrapper
    }
)))
app.listen(port, function() {
  const portAddress = this.address().port
  console.log(`Started on http://localhost:${portAddress}/`)
})

Let me know if you need anything else. Thanks

[alifaff]

My colleague has created a Github project located here: https://github.com/marc-at/exception-graphql

To run locally: npm run dev
To run at AWS serverless: sls deploy --stage testing

[IvanGoncharov]

@alifaff Nothing prevents you from writing:

const runGraphQL = (query, variables, params) => {
  const result = graphql(schema, query, null, {params}, variables);
  if (result.errors) {
    result.errors = result.errors.map(graphqlErrorManager.manageGraphErrors);
  }
}

Which is exactly what express-graphql doing internaly see here:
https://github.com/graphql/express-graphql/blob/master/src/index.js#L316-L320

However, it's bad practice since GraphQL errors should follow the certain structure:
http://facebook.github.io/graphql/draft/#sec-Errors
So you shouldn't remove message, locations and path properties. Moreover according to the latest version:

GraphQL services should not provide any additional entries to the error format since they could conflict with additional entries that may be added in future versions of this specification.

You can add additional properties only inside extensions as shown here:
http://facebook.github.io/graphql/draft/#example-fce18

The ideal solution would be to do this:

class ApiError extends Error {
  constructor(type, ...errors) {
    super(...errors)
    if (Error.captureStackTrace) {
      Error.captureStackTrace(this, ApiError)
    }
    this.extensions = {
      date: new Date(),
      type,
      errors
    };
  }
}

GraphQL library recognise extensions field inside your error and copies it inside responce.

[alifaff]

Thank you for the quick feedback. We have updated the code in Github to reflect the change to ApiError. Unfortunately, that still does not return the error back properly. We still get back empty errors:

{
    "errors": [
        {}
    ],
    "data": {
        "helloError": null
    }
}

Will you please take a quick look at the simple example project we have created to https://github.com/marc-at/exception-graphql

To run locally: npm run dev
To run at AWS serverless: sls deploy --stage testing

[IvanGoncharov]

@alifaff Works for me:

[IvanGoncharov]

@alifaff Or should I call it differently?
Anyway I checked your code and you need to add:

        if (result && result.errors) {
          result.errors = result.errors.map(formatError);
        }

Before returning result.
I'm not familiar with serverless but maybe you also need to call JSON.stringify somewhere.

[alifaff]

Hi Ivan!

That is correct that I have to stringify or return a string when throwing a GraphQLError for it to come through, however we used to use version 0.11.2 and that would return back the object. Yes it works with express-graphql but not using AWS Lambda. I believe something has changed in version 13.2 with GraphQLError that does not allow us to wrap the custom error with GraphQLError when running it in Lambda/node server. Could this some sort of bug on this version?

I am going to try all of the different version between 0.11.2 and 0.13.2 and see which version began returning an empty set. I will return back with the version shortly. Thanks

[alifaff]

Quickly ran different versions and found the last version 0.11.7 worked fine, however in version 0.12.0, it returns empty.

[IvanGoncharov]

@alifaff Could you please run this code on 0.11.7 and 0.12.0:

const result = execute(...);
console.log(JSON.stringify(result.errors[0]))

And post results here.

[IvanGoncharov]

@alifaff Also please copy the result of npm ls graphql.
Maybe you are using code from two different versions of GraphQL.

[alifaff]

v0.12.0:seems to select 0.12.3
npm ls graphql:
exception-graphql@1.0.0 /Users/ali/microservices/exception-graphql
├── graphql@0.12.3
└─┬ serverless@1.26.1
├─┬ apollo-client@1.9.3
│ ├─┬ apollo-link-core@0.5.4
│ │ └── graphql@0.10.5
│ └── graphql@0.10.5
└── graphql@0.10.5
console.log is as follows:
{}

v0.11.7:
npm ls graphql:
exception-graphql@1.0.0 /Users/ali/microservices/exception-graphql
├── graphql@0.11.7
└─┬ serverless@1.26.1
├─┬ apollo-client@1.9.3
│ ├─┬ apollo-link-core@0.5.4
│ │ └── graphql@0.10.5
│ └── graphql@0.10.5
└── graphql@0.10.5
console.log is as follows:
{
"message": {
"extensions": {
"date": "2018-04-26T14:43:05.485Z",
"type": "INPUT",
"errors": [
{
"key": "DUPLICATE",
"message": "Duplicate quote request."
}
]
}
},
"locations": [
{
"line": 1,
"column": 3
}
],
"path": [
"helloError"
]
}

[IvanGoncharov]

throw new GraphQLError(new ApiError

@alifaff You shouldn't wrap it with GraphQLError just throw new ApiError(

class ApiError extends Error {
  constructor(type, ...errors) {
    super(...errors)

throw new GraphQLError(new ApiError('INPUT', {key: 'DUPLICATE', message: `Duplicate quote request.`}))

Why are you doing that: super(...errors)?
You should pass string with error description not an object.

console.log is as follows:

This library works as expected. You shouldn't just randomly pass arguments to function/constructors please read documentation first.
For example first argument of new Error( should always be a string:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error

[alifaff]

@IvanGoncharov The constructor does have string as the first argument, in our case 'type' is string. The usage with new ApiError('Input' where 'Input' is a string. I understand that it will work with new Error(somestring), however we want to return a custom error with the type of error, root cause of the error, message to be returned to the client, and message to display to the end user. Since we want to customize the error we built our custom Error which does follow the guideline when Error needs to be extended.

I guess another way to solve the problem is to ask how do we send custom error messages back that are not only string specific but has some allowable structure to it?

[alifaff]

I suspect a problem with serverless and possibly apollo-client and graphql versions where the graphql version differ and for one reason or another it drops everything but the message in errors.

├── graphql@0.12.3
└─┬ serverless@1.26.1
├─┬ apollo-client@1.9.3
│ ├─┬ apollo-link-core@0.5.4
│ │ └── graphql@0.10.5
│ └── graphql@0.10.5
└── graphql@0.10.5

[IvanGoncharov]

The constructor does have string as the first argument, in our case 'type' is string.

@alifaff I'm talking about standard Error class and you call its constructor through super.
I strongly suggest you read JS docs about Error class:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error

[alifaff]

Thanks...I have tried everything you have provided however nothing seems to be working. As mentioned it was working up until version 11.7 and something changed in version 12.0. Anything that might ring a bell in terms of difference between the two versions that could have potentially altered the output. Problem is related to throwing an error with an object as opposed to a string which works just fine and seems to be caused when using graphql natively or in a node server.

[IvanGoncharov]

@alifaff From the result of:

const result = execute(...);
console.log(JSON.stringify(result.errors[0]))

I see that library works as expected. That mean problems are in your code.
I reviewed your sample project and point out all potential problem I saw there.

So the only way I can help you if you create sample code that I can run locally.
Since your sample project works as expected when I run it using npm run dev and I don't have the possibility to deploy/debug code on AWS.

[alifaff]

@IvanGoncharov Tried to see if we can run it through serverless locally and it works, however when we run it through AWS Lambda the errors are empty. I am not sure how we can get you test it locally without actually running it in Lambda. For the time being I will close the ticket, but we will see what change is making Lambda break. Thank you for all of your help, we really appreciate it!

[marc-at]

It turns out the reason we were seeing different results when executing on lambda was the way we were building the response.

const createResponse = (statusCode, body) => ({
  statusCode,
  headers: {
    'Access-Control-Allow-Origin': '*'
  },
  body: JSON.stringify(body)
})

JSON.stringify(body) attempts to stringify the Error and, by design, it doesn't include all the extra properties. A few solutions are discussed on SO.

[IvanGoncharov]

JSON.stringify(body) attempts to stringify the Error and, by design, it doesn't include all the extra properties. A few solutions are discussed on SO.

@marc-at Entire GraphQL response should be serializable can you please post here a sample code similar to this:

const schema = ...
const query = ...
execute(schema, query)
  .then(result => console.log(JSON.stringify(result));

And its actual and expect results.

If you speaking of missing extensions property, it's already fixed in #1284
Meanwhile, you can use code that I suggested earlier:

        if (result && result.errors) {
          result.errors = result.errors.map(formatError);
        }

[marc-at]

Thanks. That worked.

[alifaff]

Thank you very much @IvanGoncharov for helping us walk through this problem. Really appreciate all of your support!