returnData does not resolve external values nor errors

[mjbcopland]

Long overdue followup to ardatan/graphql-tools#2214

TLDR

Specifying returnData breaks resolver merging of @graphql-tools/delegate, causing errors like enum cannot represent value and silently ignoring actual errors.

Steps to reproduce

Repo here if you want to just run it (see the readme), but I will describe the steps in detail as well.

Setup

I've chosen the gRPC handler because it allows me to define enums with numeric values (which will become important later) but also because that's what I'm using when encountering this bug in the real world. The service definition is rather simple; just an enum and and object type with a value of that enum. The implementation is trivial with an input argument throw to conditionally return an error instead.

syntax = "proto3";

service Service {
  rpc getObject(Input) returns (Object) {}
}

enum Value { UNKNOWN = 0; }

message Input { string throw = 1; }
message Object { Value value = 1; }

{
  sources: [
    {
      name: "Source",
      handler: {
        grpc: {
          endpoint: "localhost:50051",
          packageName: "",
          protoFilePath: require.resolve("service/src/api.proto"),
          serviceName: "Service",
        },
      },
    },
  ],
}

The mesh converts this to GraphQL and allows calling the resolver as expected.

{
  object: getObject {
    value
  }
  error: getObject(input: {throw: "true"}) {
    value
  }
}

{
  "errors": [
    {
      "message": "2 UNKNOWN: test",
      "path": ["error"]
    }
  ],
  "data": {
    "object": {
      "value": "UNKNOWN"
    },
    "error": null
  }
}

However the problem occurs when we introduce some additions to the schema. I'm going to add a new resolver which meshes the existing resolver to return a value directly. I should now be able to make a very similar query as above.

Input

{
  additionalTypeDefs: `
    extend type Query {
      getValue: Value
    }
  `,
  additionalResolvers: [
    {
      type: "Query",
      field: "getValue",
      targetSource: "Source",
      targetMethod: "getObject",
      returnData: "value",
      args: { "input.throw": "{args.input.throw}" },
    },
  ],
}

{
  value: getValue
  error: getValue(input: {throw: "true"})
}

Expected

The result should be the same as above except without the object wrapper.

{
  "errors": [
    {
      "message": "2 UNKNOWN: test",
      "path": ["error"]
    }
  ],
  "data": {
    "value": "UNKNOWN",
    "error": null
  }
}

Actual

Instead, our enum now errors, and the expected error has disappeared.

{
  "errors": [
    {
      "message": "Enum \"Value\" cannot represent value: \"UNKNOWN\"",
      "path": ["value"]
    }
  ],
  "data": {
    "value": null,
    "error": null
  }
}

What's happening?

1. We're trying to resolve the enum using its external string value, not the internal numeric value.
2. We're returning the value property on an error object, which is undefined and hides the original error.

@graphql-tools/delegate defines special symbols used to annotate external objects. This is then used by defaultMergedResolver to resolve external values. Using lodash's get() to extract returnData from meshed results causes both issues.

Possible solutions

1. Return errors as-is and explicitly resolve external data instead of naively using get().
2. Extract returnData via a schema transform, eg "[WrapQuery] is used to get a result nested inside other result".

I've managed to achieve option 1 via patch-package in the reproduction repo linked above. It doesn't get error locations/paths correct so I'm wondering if option 2 would be more robust, but I haven't tried it yet. Note that I also had to patch merger-stitching to bypass mergeSingleSchema() as it wasn't setting stitchingInfo which is required to resolve external values.