mercurius icon indicating copy to clipboard operation
mercurius copied to clipboard

Ambiguous error message 400

Open lewispham opened this issue 2 years ago • 5 comments

After upgrading to version 8.2.1, I'm receiving this error for all syntax errors and also the query depth error.

{
    "statusCode": 400,
    "code": "MER_ERR_GQL_VALIDATION",
    "error": "Bad Request",
    "message": "Graphql validation error"
}

This error is obviously not enough for debugging.

After double-checking the issue then it's because of my custom error formatter.

            errorFormatter: (result: ExecutionResult) => {
                let statusCode = 500;
                if (result.errors) {
                    const firstError = result.errors[0];
                    if (firstError.extensions) {
                        statusCode = firstError.extensions.statusCode;
                    }
                }
                return {
                    statusCode,
                    response: result
                };
            }

I still can not figure out the issue of that error formatter function. It's only for changing the http status code. But removing it solves this issue.

lewispham avatar Sep 20 '21 10:09 lewispham

Which was the last released that worked fine for you?

I think you might want to add some logging to your errorFormatter.

mcollina avatar Sep 20 '21 10:09 mcollina

@mcollina It's v7.4.0 . Which sort of logs do you need? I tried to log result.errors and the error is the correct one.

lewispham avatar Sep 20 '21 11:09 lewispham

What is the correct error?

Could you please upload an example to reproduce?

There is nothing that strikes me as changing this between v7.4.0 and v7.5.0.

mcollina avatar Sep 20 '21 12:09 mcollina

@mcollina I've just created this repo for reproducing the issue. https://github.com/lewispham/mercurius-572 You can follow the readme to reproduce it.

lewispham avatar Sep 20 '21 13:09 lewispham

Take a look at https://github.com/mercurius-js/mercurius/blob/037cc1f35c3e4901d3dc219a9ef88b43ac017b0e/lib/errors.js#L63-L94.

You should implement the errorFormatter in a different way if you want all the errors, you do not need to return an Error instance but rather a "plain object" that could be easily serialized. Essentially result is an error.

Take a look at:

app.register(mercurius, {
  schema,
  resolvers,
  errorFormatter: (result) => {
    const def = mercurius.defaultErrorFormatter(result)

    let statusCode = 500;
    if (result.errors) {
        const firstError = result.errors[0];
        if (firstError.extensions) {
            statusCode = firstError.extensions.statusCode;
        }
    }
    const res = {
        statusCode,
        response: result
    };

    console.log(JSON.stringify(def))
    console.log(JSON.stringify(res))

    return JSON.parse(JSON.stringify(res))
  }
})

mcollina avatar Sep 21 '21 14:09 mcollina

It works for me, but the current version requires an extra context arguments. This issue should be solved but I think we need to document this in the errorFormatter section.

import mercurius from 'mercurius';
const { defaultErrorFormatter } = mercurius;

fastify.register(mercurius, {
    schema,
    errorFormatter: (result, ctx) => {
      const def = defaultErrorFormatter(result, ctx);

      return {
        statusCode: def.statusCode || 500,
        response: def.response,
      };
    },
});

linkthai avatar Oct 11 '23 04:10 linkthai

@linkthai would you mind to send a PR to update the docs?

mcollina avatar Oct 11 '23 09:10 mcollina

@mcollina alright, I made a pull request here #1034

linkthai avatar Oct 12 '23 03:10 linkthai