Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Grmiade opened this issue Nov 16, 2021 · 2 comments
Open

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

Grmiade opened this issue Nov 16, 2021 · 2 comments
Labels
stage/1-reproduction A reproduction exists

Comments

@Grmiade
Copy link

Grmiade commented Nov 16, 2021

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 apollographql/federation#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.

@ardatan
Copy link
Owner

ardatan commented Jan 13, 2022

Would you create a PR for this failing test :)

@ardatan ardatan added the stage/1-reproduction A reproduction exists label Jan 13, 2022
@RIP21
Copy link

RIP21 commented Dec 10, 2022

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stage/1-reproduction A reproduction exists
Projects
None yet
Development

No branches or pull requests

3 participants