mogelbrod commented 1 year ago

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:

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:

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
```

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

git clone https://github.com/mogelbrod/graphql-codegen-issue
cd graphql-codegen-issue
npm install
graphql-codegen
Expect: graphql-codegen reports 1 error that this line/query has invalid GraphQL syntax
Open src/index.ts and comment out this line
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/cli@3.2.2
├── @graphql-codegen/fragment-matcher@4.0.1
├── @graphql-codegen/gql-tag-operations-preset@2.1.0
├── @graphql-codegen/typescript-apollo-client-helpers@2.2.6
├── @graphql-typed-document-node/core@3.1.2
└── graphql@16.6.0

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

bradleyayers commented 1 year ago

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

mogelbrod commented 1 year ago

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 🤔

ardatan commented 1 year ago

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.

bradleyayers commented 1 year ago

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.

erquhart commented 1 year ago

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?

tnyo43 commented 1 year ago

@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,
  ...
};

duffytilleman commented 11 months ago

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.

egucciar commented 1 month ago

even with noSilentErrors set to true, it is not catching the following syntax issue:

    noSilentErrors: true,
    ignoreNoDocuments: true, // for better experience with the watcher
    generates:

ntinkler-calendly commented 2 weeks ago

I can reproduce this consistently when ignoreNoDocuments: true is enabled even with noSilentErrors: true. If the ignoreNoDocuments flag is set I never see any error output for invalid documents. Instead it just happily completes as though nothing is wrong.

If you set it to ignoreNoDocuments: false, you'll see error output (although I don't find it particularly helpful, given it doesn't include the file/line info at all). But at least the thing fails instead of just merrily claiming that everything is fine.

I also can't get errorsOnly: true to do jack squat - the tool spits out a bunch of status text either way.

lexome-neal commented 4 days ago

perhaps a happy middle ground here is to make the default behavior to log in the console which files are encountering errors, even if it doesn't break the build. silencing this log could be a opt-in config option. or at least logging which files are being skipped and why when VERBOSE is set to true.

this github ticket being the only place where this behavior is documented is not a great developer experience.