ardatan / graphql-tools

:wrench: Utility library for GraphQL to build, stitch and mock GraphQL schemas in the SDL-first approach
https://www.graphql-tools.com
MIT License
5.35k stars 815 forks source link

wrapSchema transforms the response when error path is not provided #3880

Open Grmiade opened 2 years ago

Grmiade commented 2 years ago

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

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

ardatan commented 2 years ago

Would you create a PR for this failing test :)

RIP21 commented 1 year ago

@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",