defaultErrorFormatter throws error for federated errors (when downstream service can't be reached)

[tomi-bigpi]

Ran into the following issue (locally/in development) using Mercurius in gateway mode to a downstream Mercurius API that isn't running. The error I'm seeing is error.extensions.errors.map is not a function.

The relevant code https://github.com/mercurius-js/mercurius/blob/master/lib/errors.js#L57 expects error.extensions.errors to be an array:

      if (error.message === FEDERATED_ERROR.toString() && error.extensions) {
        return error.extensions.errors.map(err => formatError(toGraphQLError(err)))
      }

However, in reality is just an object with {"errno":-61,"code":"ECONNREFUSED","syscall":"connect","address":"127.0.0.1","port":1234} or toStringed:

Error: connect ECONNREFUSED 127.0.0.1:4412
    at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1158:16) {
  errno: -61,
  code: 'ECONNREFUSED',
  syscall: 'connect',
  address: '127.0.0.1',
  port: 1234
}

Or looking at the top level error:

{
  "message": "Symbol(FEDERATED_ERROR)",
  "locations": [{
      "line": 2,
      "column": 5
    }],
  "path": [ "me" ],
  "extensions": {
    "errors": {
      "errno": -61,
      "code": "ECONNREFUSED",
      "syscall": "connect",
      "address": "127.0.0.1",
      "port": 1234
    }
  }
}

Sample snippet to reproduce with alternate simple custom formatter commented out to log the object that can be used as a simplified workaround:

    const schema = `
      extend type Query {
        me: User
      }

      type User @key(fields: "id") {
        id: ID!
        name: String
        username: String
      }
    `;

    app.register(mercurius, {
      // errorFormatter: (result) => {
      //   console.log(result.errors && JSON.stringify(result.errors[0].extensions.errors));
      //   return {
      //     response: result,
      //     statusCode: 500,
      //   };
      // },
      gateway: {
        services: [
          {
            name: "user",
            schema,
            url: "http:/127.0.1:1234/graphql",
          },
        ],
      },
    });

[mcollina]

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

[tomi-bigpi]

I created the above PR to address the issue and help ensure it doesn't accidentally happen elsewhere by validating the input for FederatedError.

One thing that is up for discussion is the extension of the options for services that allows passing in a undici agent instance. This was based on the recommendation in this open issue: nodejs/undici#996 The other option I thought about was monkeypatching with Sinon stubs, but went with the undici recommendation here. Let me know if you've got another method you'd like to use to mock out the request in the tests.

Note and likely unrelated: One of the existing tests (gateway - should log error if retry throws) was failing for me locally (macOS 12.2.1, Node 17.6.0 and Node 16.14.0 which was used in tests by Github). It was failing on the master branch without any modifications, so not sure what's going on there. Maybe a dependency had some minor version change? I couldn't tell since package-lock.json wasn't available.

[mcollina]

I created the above PR to address the issue and help ensure it doesn't accidentally happen elsewhere by validating the input for FederatedError.

Great!

One thing that is up for discussion is the extension of the options for services that allows passing in a undici agent instance. This was based on the recommendation in this open issue: nodejs/undici#996 The other option I thought about was monkeypatching with Sinon stubs, but went with the undici recommendation here. Let me know if you've got another method you'd like to use to mock out the request in the tests.

I think the PR is ok as it is, thanks!

Note and likely unrelated: One of the existing tests (gateway - should log error if retry throws) was failing for me locally (macOS 12.2.1, Node 17.6.0 and Node 16.14.0 which was used in tests by Github). It was failing on the master branch without any modifications, so not sure what's going on there. Maybe a dependency had some minor version change? I couldn't tell since package-lock.json wasn't available.

Tests seems passing on CI (but they are flaky from time to time). Would you like to send a different PR to address this?

[tomi-bigpi]

re: The failing test. I wasn't able to get that test to pass and I don't have the time to troubleshoot it at the moment. Since it's passing on CI, we can probably ignore for the time being.