apollographql / apollo-utils

Monorepo of common utilities related to Apollo and GraphQL
MIT License
36 stars 9 forks source link

`generate-persisted-query-manifest` : Hard to debug violations; Can docs with multiple root-level mutations be allowed? #431

Closed samjcombs closed 4 months ago

samjcombs commented 4 months ago

Recently while generating a persisted query manifest for a project with 400+ files to scan for operations, I got the following error:

Invariant Violation: An error occurred! For more details, see the full error text at https://go.apollo.dev/c/err#%7B%22version%22%3A%223.9.11%22%2C%22message%22%3A76%2C%22args%22%3A%5B%222%22%5D%7D
    at new InvariantError (/my-app/node_modules/@apollo/client/node_modules/ts-invariant/lib/invariant.cjs:16:28)
    at Object.invariant (/my-app/node_modules/@apollo/client/node_modules/ts-invariant/lib/invariant.cjs:28:15)
    at Object.invariant (/my-app/node_modules/@apollo/client/utilities/globals/globals.cjs:64:21)
    at checkDocument (/my-app/node_modules/@apollo/client/utilities/utilities.cjs:461:13)
    at DocumentTransform.performWork (/my-app/node_modules/@apollo/client/utilities/utilities.cjs:575:9)
    at DocumentTransform.transformDocument (/my-app/node_modules/@apollo/client/utilities/utilities.cjs:582:40)
    at QueryManager.transform (/my-app/node_modules/@apollo/client/core/core.cjs:1681:39)
    at QueryManager.query (/my-app/node_modules/@apollo/client/core/core.cjs:1740:99)
    at ApolloClient.query (/my-app/node_modules/@apollo/client/core/core.cjs:2477:34)
    at generatePersistedQueryManifest (/my-app/node_modules/@apollo/generate-persisted-query-manifest/dist/index.js:332:30) {
  framesToPop: 1
}

It was confusing that I was receiving an Apollo Client error. I followed the link to the error page and found this:

image

With over 400 files with potential operations to choose from, this provided no insight into where the problem was.

I was able to find the culprit by following the strace to here.

By adding a conditional breakpoint on a this line (operations.length > 1) I was able to get the operation document, which I then used to search my project and find the culprit, which ended up being two mutations side by side that looked similar to this:

graphqlOperations.mutations = gql`
  mutation RegisterItem(
    $input: RegisterItemInput
  ) {
    registerItem(input: $input) {
      status
      message
    }
  }
  mutation SyncItem(
    $input: SyncItemInput
  ) {
    syncItem(input: $input) {
      isSuccess
      message
    }
  }
`;

The above method of debugging "invariant violations" is not ergonomic for users. It would be great if the operation name and the filename/location were surfaced here.

Also, in this particular case, calling two root-level mutations synchronously is valid according to the GraphQL spec. I would expect this operation not to have surfaced a violation.

Is there a workaround to allow generation of persisted query manifest entries for documents containing multiple root level mutations?

gwardwell commented 4 months ago

We have use cases where we perform both multiple mutations and multiple queries in a single operation document. This is definitely a use case we would need supported in order to adopt persisted queries.

glasser commented 4 months ago

From a discussion privately with @samjcombs and @gwardwell it sounds like multiple operations per document aren't actually required here (note that Apollo Client Web does not support this anyway, and this tool is specifically for the ACW use case) — but the error handling is certainly in need of improvement in any case.

gwardwell commented 4 months ago

Thanks @glasser. As you noted, my use case is actually multiple top level mutation/query fields with is supported, not multiple queries or mutations which I don't need to be supported. Thanks for looking at the error messaging!

jerelmiller commented 4 months ago

Hey all 👋

Thanks for your patience here!

I've got a PR up (https://github.com/apollographql/apollo-utils/pull/435) that should fix this going forward. It now detects multiple operations in a single document and will report those as errors as well as making sure that errors that might originate from the client instance are not obscured which should make it easier to detect the source of the error. Hope this helps moving forward! Thanks for reporting the issue.

samjcombs commented 4 months ago

Thanks much!