graphql-tools

graphql-tools copied to clipboard

Describe the bug

The wrapSchema function transforms the response when error path is not provided. Currently, @apollo/gateway doesn't provide this top-level path, so when we wrap a federated graph, responses with errors are not handled correctly.

See https://github.com/apollographql/federation/issues/354

To Reproduce

Here a test suite to reproduce the issue:

import { makeExecutableSchema } from '@graphql-tools/schema'
import { wrapSchema } from '@graphql-tools/wrap'
import { graphql } from 'graphql'

function generateResponse(withPath = false): Record<string, unknown> {
  return {
    errors: [
      {
        message: 'Country not found',
        path: withPath ? ['companies', 1, 'country', 'name'] : undefined,
      },
    ],
    data: {
      companies: [
        {
          country: null,
        },
        {
          country: {
            name: null,
          },
        },
        {
          country: {
            name: 'France',
          },
        },
      ],
    },
  }
}

const typeDefs = `
  type Country {
    code: String!
    name: String
  }

  type Company {
    country: Country
  }

  type Query {
    companies: [Company!]!
  }
`

const resolvers = {
  Country: {
    name(parent: { code: string }) {
      switch (parent.code) {
        case 'FR':
          return 'France'
        case 'US':
          return 'United States'
        default:
          throw new Error('Country not found')
      }
    },
  },
  Query: {
    companies() {
      return [{ country: null }, { country: { code: 'BE' } }, { country: { code: 'FR' } }]
    },
  },
}

const companySchema = makeExecutableSchema({ resolvers, typeDefs })

it('should success with companySchema', async () => {
  const response = await graphql(
    companySchema,
    `
      {
        companies {
          country {
            name
          }
        }
      }
    `,
  )

  expect(response.data?.companies[0].country).toBe(null)
  expect(response.data?.companies[1].country).toEqual({ name: null })
  expect(response.errors).toBeDefined()
})

it('should success by wrapping companySchema when path is provided', async () => {
  const wrappedCompanySchema = wrapSchema({
    executor() {
      return generateResponse(true)
    },
    schema: companySchema,
  })

  const response = await graphql(
    wrappedCompanySchema,
    `
      {
        companies {
          country {
            name
          }
        }
      }
    `,
  )

  expect(response.data?.companies[0].country).toBe(null)
  expect(response.data?.companies[1].country).toEqual({ name: null })
  expect(response.errors).toBeDefined()
})

it('should success by wrapping companySchema when path is not provided', async () => {
  const wrappedCompanySchema = wrapSchema({
    executor() {
      return generateResponse(false)
    },
    schema: companySchema,
  })

  const response = await graphql(
    wrappedCompanySchema,
    `
      {
        companies {
          country {
            name
          }
        }
      }
    `,
  )

  console.log(JSON.stringify(response, null, 2))

  expect(response.data?.companies[0].country).toBe(null)
  expect(response.data?.companies[1].country).toEqual({ name: null })
  expect(response.errors).toBeDefined()
})

Current behavior

When the error doesn't provide the path, we receive:

{
  "data": {
    "companies": [
      {
        "country": {
          "name": null
        }
      },
      {
        "country": {
          "name": null
        }
      },
      {
        "country": {
          "name": "France"
        }
      }
    ]
  }
}

Is this behavior is wanted?

Expected behavior

We should probably receive:

{
  "errors": [{ "message": "Country not found" }],
  "data": {
    "companies": [
      {
        "country": null
      },
      {
        "country": {
          "name": null
        }
      },
      {
        "country": {
          "name": "France"
        }
      }
    ]
  }
}

Environment

• @graphql-tools/schema to 8.3.1
• @graphql-tools/wrap to 8.3.2
• graphql to 15.7.2

Thanks in advance 🙏 Let me know if you need more details.

Would you create a PR for this failing test :)

@ardatan Schema wrap not only transforms the swallow location but also swallows the original error type somehow. Stitching is the same. Also, all regular errors (e.g. throw new Error("Should be redacted")) are considered GraphQLError, which means that Error Masking in Yoga v3 doesn't work. Below are my versions.

    "graphql": "~16.6.0",
    "graphql-tools": "~8.3.14",
    "graphql-yoga": "~3.1.1",