errorFormatter has wrong typescript types

[fabulator]

errorFormatter in options has following typescript definition:

interface ExecutionResult<
  TData = ObjMap<unknown>,
  TExtensions = ObjMap<unknown>,
> {
  errors?: ReadonlyArray<GraphQLError>;
  data?: TData | null;
  extensions?: TExtensions;
}

errorFormatter?: <TContext extends MercuriusContext = MercuriusContext>(
    execution: ExecutionResult,
    context: TContext
  ) => {
    statusCode: number;
    response: ExecutionResult;
  };

But execution doesn't always get ExecutionResult. It can also get an Error type. Here is an example:

import Fastify from 'fastify';
import mercurius from 'mercurius';

const app = Fastify()

const schema = `
  type Query {
    field: Int
  }
`;

app.register(mercurius, {
    path: '/',
    schema,
    errorFormatter: (response) => {
        // @ts-ignore
        console.log(response.code);
        return { statusCode: 200, response }
    }
})

app.listen({ port: 3000 })

When I run the app and send GET to / it will log MER_ERR_GQL_PERSISTED_QUERY_NOT_FOUND which doesn't corespond with types.

I'm not sure if bug is wrong type (that there should be union with Error) or that Error shouldn't by passed here.

const defaultErrorFormatter: (
    execution: ExecutionResult | Error,
    context: MercuriusContext
  ) => { statusCode: number; response: ExecutionResult };

But the types for the defaultErrorFormatter have Error as union option. So fix might be only changing the type.

[mcollina]

Thanks for reporting! Would you like to send a Pull Request to address this issue? Remember to add unit tests.

There might be a bug in the code, as I think we should expect an ExecutionResult. What would you recommend as a fix?

[fabulator]

@mcollina I checked the code and I think I know where is the problem. The defaultErrorFormatter has following:

let errors = [err instanceof ErrorWithProps ? formatError(toGraphQLError(err)) : { message: err.message }]

Basically the first line do the all transformation to handle the Error type.

I could try to move it out of the default function and put it into the lib/routes.js. The errorFormatter would get transformed results without an Error Type.

I would also have to correct the state, when error is not instance of ErrorWithProps and it is transformed to { message: err.message }. This doesn't satisfy typescript type ReadonlyArray<GraphQLError> which is specified in execution type.

If it sounds OK I could work on PR.

[mcollina]

Thanks for the detailed explanation, and go for it!

[fabulator]

I haved created the PR https://github.com/mercurius-js/mercurius/pull/879

[ilteoood]

I think that this can be closed now

[simoneb]

Thanks @ilteoood