graphql-code-generator icon indicating copy to clipboard operation
graphql-code-generator copied to clipboard

No error output whatsoever when parsed GraphQL documents have syntax errors

Open mogelbrod opened this issue 1 year ago • 7 comments

Which packages are impacted by your issue?

@graphql-codegen/cli, @graphql-codegen/core, @graphql-codegen/gql-tag-operations-preset

Describe the bug

Likely related issues:

  • https://github.com/dotansimha/graphql-code-generator/issues/6749
  • https://github.com/dotansimha/graphql-code-generator/issues/8841

Running graphql-codegen in a project containing invalid GraphQL queries in typescript files wrapped with gql() has some really strange and inconsistent behaviour with regards to error reporting:

  1. If only src/index.ts OR src/nested/directory/file.ts contains an invalid query it doesn't get reported
     $ graphql-codegen
     ✔ Parse Configuration
     ✔ Parse Configuration
     ✔ Generate outputs
    
  2. If both src/index.ts AND src/nested/directory/file.ts contain invalid queries both errors are reported, but with no information regarding which file, line, or query
     $ graphql-codegen
     ✔ Parse Configuration
     ⚠ Generate outputs
       ❯ Generate to src/graphql/
         ✔ Load GraphQL schemas
         ✖ Failed to find any GraphQL type definitions in: src/**/*.{graphql,ts,tsx,js,jsx};
           - Syntax Error: Expected Name, found "}".
           - Syntax Error: Expected Name, found "}".
         ◼ Generate
    

Running the graphql-codegen command with --debug and/or --verbose flags changes nothing, but prepending the command with DEBUG=1 does result in an error being logged in both cases, albeit with a bunch of noise surrounding it:

DEBUG=1 graphql-codegen --verbose output
[STARTED] Parse Configuration
[SUCCESS] Parse Configuration
[STARTED] Generate outputs
[STARTED] Generate to src/graphql/
[STARTED] Load GraphQL schemas
@graphql-tools/load: normalizePointers [object Object]: 0.058ms
@graphql-tools/load: normalizePointers: 0.172ms
@graphql-tools/load: collectDocumentString https://rickandmortyapi.com/graphql: 0.283ms
@graphql-tools/load: collectSources https://rickandmortyapi.com/graphql: 0.408ms
@graphql-tools/load: loadFile https://rickandmortyapi.com/graphql: 213.892ms
@graphql-tools/load: collectFallback https://rickandmortyapi.com/graphql: 214.071ms
@graphql-tools/load: collectSources queue: 214.26ms
@graphql-tools/load: parseSchema https://rickandmortyapi.com/graphql: 3.482ms
@graphql-tools/load: parseRawSDL https://rickandmortyapi.com/graphql: 1.608ms
@graphql-tools/load: useComments https://rickandmortyapi.com/graphql: 0.004ms
@graphql-tools/load: collectValidSources https://rickandmortyapi.com/graphql: 0.013ms
@graphql-tools/load: parseSource https://rickandmortyapi.com/graphql: 5.695ms
@graphql-tools/load: prepareResult: 0.004ms
@graphql-tools/load: loadTypedefs: 221.377ms
[SUCCESS] Load GraphQL schemas
[STARTED] Load GraphQL documents
@graphql-tools/load: normalizePointers [object Object]: 0.008ms
@graphql-tools/load: normalizePointers: 0.15ms
@graphql-tools/load: collectDocumentString src/**/*.{graphql,ts,tsx,js,jsx}: 0.154ms
@graphql-tools/load: collectSources src/**/*.{graphql,ts,tsx,js,jsx}: 0.186ms
(node:11392) Warning: Label '@graphql-tools/load: collectSources' already exists for console.time()
(Use `node --trace-warnings ...` to show where the warning was created)
Failed to load schema from code file "/Users/victor/src/graphql-codegen-issue/src/index.ts": Syntax Error: Expected Name, found "}".
GraphQLError: Syntax Error: Expected Name, found "}".
    at syntaxError (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/error/syntaxError.js:15:10)
    at Parser.expectToken (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1397:40)
    at Parser.parseName (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:108:24)
    at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:347:30)
    at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
    at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26)
    at Parser.parseSelectionSet (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:320:24)
    at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:365:16)
    at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
    at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26) {
  path: undefined,
  locations: [ { line: 4, column: 7 } ],
  extensions: [Object: null prototype] {}
}
Failed to load schema from code file "/Users/victor/src/graphql-codegen-issue/src/nested/directory/file.ts": Syntax Error: Expected Name, found "}".
GraphQLError: Syntax Error: Expected Name, found "}".
    at syntaxError (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/error/syntaxError.js:15:10)
    at Parser.expectToken (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1397:40)
    at Parser.parseName (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:108:24)
    at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:347:30)
    at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
    at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26)
    at Parser.parseSelectionSet (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:320:24)
    at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:365:16)
    at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
    at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26) {
  path: undefined,
  locations: [ { line: 6, column: 7 } ],
  extensions: [Object: null prototype] {}
}
AggregateError: Reading from src/**/*.{graphql,ts,tsx,js,jsx} failed ; 
 Syntax Error: Expected Name, found "}".
Syntax Error: Expected Name, found "}".
    at CodeFileLoader.load (/Users/victor/src/graphql-codegen-issue/node_modules/@graphql-tools/code-file-loader/cjs/index.js:109:19)
    at async /Users/victor/src/graphql-codegen-issue/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:17:39
    at async Promise.all (index 1)
    at async loadFile (/Users/victor/src/graphql-codegen-issue/node_modules/@graphql-tools/load/cjs/load-typedefs/load-file.js:15:9)
    at async /Users/victor/src/graphql-codegen-issue/node_modules/@graphql-tools/load/cjs/load-typedefs/collect-sources.js:229:25 {
  [errors]: [
    GraphQLError: Syntax Error: Expected Name, found "}".
        at syntaxError (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/error/syntaxError.js:15:10)
        at Parser.expectToken (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1397:40)
        at Parser.parseName (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:108:24)
        at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:347:30)
        at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
        at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26)
        at Parser.parseSelectionSet (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:320:24)
        at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:365:16)
        at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
        at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26) {
      path: undefined,
      locations: [Array],
      extensions: [Object: null prototype] {}
    },
    GraphQLError: Syntax Error: Expected Name, found "}".
        at syntaxError (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/error/syntaxError.js:15:10)
        at Parser.expectToken (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1397:40)
        at Parser.parseName (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:108:24)
        at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:347:30)
        at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
        at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26)
        at Parser.parseSelectionSet (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:320:24)
        at Parser.parseField (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:365:16)
        at Parser.parseSelection (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:337:14)
        at Parser.many (/Users/victor/src/graphql-codegen-issue/node_modules/graphql/language/parser.js:1511:26) {
      path: undefined,
      locations: [Array],
      extensions: [Object: null prototype] {}
    }
  ]
}
[FAILED] Failed to find any GraphQL type definitions in: src/**/*.{graphql,ts,tsx,js,jsx};
[FAILED]  - Syntax Error: Expected Name, found "}".
[FAILED]   - Syntax Error: Expected Name, found "}".
[SUCCESS] Generate outputs
Running lifecycle hook "afterStart" scripts...
[CLI] Loading Schemas
[CLI] Loading Documents

Your Example Website or App

https://github.com/mogelbrod/graphql-codegen-issue

Steps to Reproduce the Bug or Issue

  1. git clone https://github.com/mogelbrod/graphql-codegen-issue
  2. cd graphql-codegen-issue
  3. npm install
  4. graphql-codegen
  5. Expect: graphql-codegen reports 1 error that this line/query has invalid GraphQL syntax
  6. Open src/index.ts and comment out this line
  7. Expect: graphql-codegen reports 2 errors for invalid GraphQL syntax, one for each of the files. Each error output should also include the file path (and preferably line number) where the error occurred.

Expected behavior

See Expect: in the "Steps to reproduce" list.

Screenshots or Videos

No response

Platform

  • OS: MacOS Ventura 13.0 (22A380)
  • NodeJS: 18.13.0
graphql-codegen-issue
├── @graphql-codegen/[email protected]
├── @graphql-codegen/[email protected]
├── @graphql-codegen/[email protected]
├── @graphql-codegen/[email protected]
├── @graphql-typed-document-node/[email protected]
└── [email protected]

Codegen Config File

# graphql.config.yaml
schema: 'https://rickandmortyapi.com/graphql'
documents:
  - 'src/**/*.{graphql,ts,tsx,js,jsx}'
extensions:
  codegen:
    overwrite: true
    # silent: false
    # debug: true
    # verbose: true
    # errorsOnly: false
    ignoreNoDocuments: true
    generates:
      'src/graphql/':
        preset: gql-tag-operations-preset
        plugins:
          - typescript-apollo-client-helpers
          - fragment-matcher
        config:
          typesPrefix: I
          enumPrefix: I
          enumsAsTypes: true
          documentVariableSuffix: Doc
          fragmentVariableSuffix: Fragment
          omitOperationSuffix: true # Don't add Query/Fragment/Subscription/Mutation suffix
          dedupeFragments: true
          exportFragmentSpreadSubTypes: true
          nonOptionalTypename: true
          flattenGeneratedTypes: false # Disabled since it removes fragment types

mogelbrod avatar Mar 15 '23 13:03 mogelbrod

I dug through the code and found there's already some infrastructure in place for this, https://github.com/ardatan/graphql-tools/blob/f66eae82b4dd561d750dcd7f419c6a6da7055550/packages/loaders/code-file/src/index.ts#L34-L37.

You can enable it in your config like this:

# graphql.config.yaml
schema: 'https://rickandmortyapi.com/graphql'
documents:
  - 'src/**/*.{graphql,ts,tsx,js,jsx}'
extensions:
  codegen:
    noSilentErrors: true
#   ^^^^^^^^^^^^^^^^^^^^-------- this option 🎉
    overwrite: true
    # silent: false
    # debug: true
    # verbose: true
    # errorsOnly: false
    ignoreNoDocuments: true
    generates:
      'src/graphql/':
        preset: gql-tag-operations-preset
        plugins:
          - typescript-apollo-client-helpers
          - fragment-matcher
        config:
          typesPrefix: I
          enumPrefix: I
          enumsAsTypes: true
          documentVariableSuffix: Doc
          fragmentVariableSuffix: Fragment
          omitOperationSuffix: true # Don't add Query/Fragment/Subscription/Mutation suffix
          dedupeFragments: true
          exportFragmentSpreadSubTypes: true
          nonOptionalTypename: true
          flattenGeneratedTypes: false # Disabled since it removes fragment types

bradleyayers avatar May 04 '23 19:05 bradleyayers

Great find @bradleyayers! I can confirm that setting extensions.codegen.noSilentErrors to true makes graphql-codegen output each error it encounters. Albeit without specifying which file the error was encountered in.

The JSDoc comment for the option states the following:

Set to true to raise errors if any matched files are not valid GraphQL

I wonder why this isn't the default behaviour? Maybe due to it being a breaking change?

#3596 is the original PR that introduced this option.

Perhaps the docs should be updated to recommend this setting 🤔

mogelbrod avatar May 05 '23 07:05 mogelbrod

It is not the default behavior because we cannot assume all calls of gql have graphql operations as a string in the code file because it can have a reference from a variable or string interpolation. So I don't think docs should recommend this setting.

ardatan avatar May 05 '23 08:05 ardatan

Albeit without specifying which file the error was encountered in.

You can get the filename by re-running with DEBUG=1 environment variable set, e.g. DEBUG=1 graphql-codegen

I wonder why this isn't the default behaviour? Maybe due to it being a breaking change?

My impression is that the root of the problem is the design of the loader chain, where if a loader throws an exception it skips to the next loader, until no loaders are left.

we cannot assume all calls of gql have graphql operations as a string in the code file because it can have a reference from a variable or string interpolation.

I don't fully understand this… could you give an example where the assumption doesn't hold? My sense is that most consumers would be entirely happy if every gql string literal must be a valid graphql document.

bradleyayers avatar May 05 '23 19:05 bradleyayers

graphql-codegen silently failing has been a bit of a challenge for me as well. I use the client preset and codegen.ts for config, and it appears noSilentErrors isn't available for this setup. The debug flag works, but as mentioned previously actual errors are deeply buried.

Any way for me to enable noSilentErrors?

erquhart avatar May 23 '23 18:05 erquhart

@erquhart Hi, I created a patch to make noSilentErrors option be valid in "codegen.ts" (https://github.com/dotansimha/graphql-code-generator/pull/9523). You can now use it as like follows:

const config: CodegenConfig = {
  schema: './schema.graphql',
  noSilentErrors: true,
  ...
};

tnyo43 avatar Aug 03 '23 09:08 tnyo43

I just want to advocate for this being the default, or if not at least being encouraged in the docs. (Or at the very least mentioned in the docs). It's a huge violation of principle of least surprise for tools to have a syntax error in a gql document result in the entire file being silently ignored. This resulted in me wasting hours trying to figure out why a source file with gql queries wasn't being included.

If it's too much of a breaking change to have generations start failing due to syntax errors, the default behavior could at least be to output a warning about the syntax error, and then change the optional config variable to something like failOnGqlSyntaxError.

duffytilleman avatar Dec 20 '23 18:12 duffytilleman