#5064 Display detailed errors from CLI

[jlmessenger]

Description

All syntax errors in referenced *.graphql files resulted in the CLI displaying the generic error message:

Something went wrong

Investigation found that that the error details were captured by listr but were not printed to the CLI. This change checks for aggregate errors, and include them in the CLI error output.

Related #5064

Type of change

• Bug fix (non-breaking change which fixes an issue)

Screenshots/Sandbox (if appropriate/relevant):

Adding links to sandbox or providing screenshots can help us understand more about this PR and take action on it as appropriate

How Has This Been Tested?

Tested by running against valid and invalid .graphql files, to ensure the error details were printed out as follows:

Something went wrong
Failed to load schema for "path/to/output-types.ts"
        Failed to load schema from path/a.graphql,path/b.graphql:

        Syntax Error: Expected ":", found Name "credentialSetup".
        GraphQLError: Syntax Error: Expected ":", found Name "credentialSetup".
    at syntaxError (/workspace/node_modules/graphql/error/syntaxError.js:15:10)
    at Parser.expectToken (/workspace/node_modules/graphql/language/parser.js:1413:40)
    at Parser.parseFieldDefinition (/workspace/node_modules/graphql/language/parser.js:881:10)
    at Parser.optionalMany (/workspace/node_modules/graphql/language/parser.js:1503:28)
    at Parser.parseFieldsDefinition (/workspace/node_modules/graphql/language/parser.js:868:17)
    at Parser.parseObjectTypeExtension (/workspace/node_modules/graphql/language/parser.js:1176:23)
    at Parser.parseTypeSystemExtension (/workspace/node_modules/graphql/language/parser.js:1094:23)
    at Parser.parseDefinition (/workspace/node_modules/graphql/language/parser.js:153:23)
    at Parser.many (/workspace/node_modules/graphql/language/parser.js:1523:26)
    at Parser.parseDocument (/workspace/node_modules/graphql/language/parser.js:115:25)
    
        GraphQL Code Generator supports:
          - ES Modules and CommonJS exports (export as default or named export "schema")
          - Introspection JSON File
          - URL of GraphQL endpoint
          - Multiple files with type definitions (glob expression)
          - String in config file
    
        Try to use one of above options and run codegen again.

Checklist:

• I have followed the CONTRIBUTING doc and the style guidelines of this project
• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have made corresponding changes to the documentation
• My changes generate no new warnings
• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes
• Any dependent changes have been merged and published in downstream modules

[changeset-bot]

🦋 Changeset detected

Latest commit: 5a76e5c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages

Name	Type
@graphql-codegen/cli	Patch
@graphql-cli/codegen	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/theguild/graphql-code-generator/7gxSAUjnrLwovUb9PsL2uzLQMAce
✅ Preview: https://graphql-code-generator-git-fork-jlmessenger-cli-errors-theguild.vercel.app

[n1ru4l]

Hey @jlmessenger, could you please provide a minimal reproduction for the "Something went wrong" error message? I was trying to reproduce it locally, but could not figure it out. I just wanna make sure we are actually addressing a reproducible issue with this PR!

[n1ru4l]

Actually, I was able to reproduce this issue accidentally:

it('Should sort the input schema', async () => {
  const nonSortedSchema = /* GraphQL */ `
    type Query {
      d: String
      z: String
      a: String
    }

    type User {
      aa: String
      a: String
    }

    type A {
      s: String
      b: String
    }
  `;
  const output = await executeCodegen({
    schema: [nonSortedSchema],
    generates: {
      'out1.graphql': {
        plugins: ['schema-ast'],
      },
    },
    config: {
      sort: true,
    },
  });

  expect(output.length).toBe(1);
  expect(output[0].content).toBeSimilarStringTo(/* GraphQL */ `
    type A {
      b: String
      s: String
    }

    type Query {
      a: String
      d: String
      z: String
    }

    type User {
      a: String
      aa: String
    }
  `);
});

it('Handles weird errors due to invalid schema', async () => {
  const schema = /* GraphQL */ `
    type Query {
      brrrt:1
    }
  `;
  await executeCodegen({
    schema: [schema],
    generates: {
      'out1.graphql': {
        plugins: ['schema-ast'],
      },
    },
  });
});

I wonder whether executeCodegen should actually already return the formatted information. That would also help tremendously when writing tests. What do you think @dotansimha ?

[jlmessenger]

@n1ru4l - Yes, it was triggering Something went wrong due to a minor typo in the graphql schema file.

The actual schema tools are returning well formatted errors.

However, listr is capturing them all and throwing a generic "Something went wrong" error:
https://github.com/SamVerschueren/listr/blob/f25152bc5291db2319a4c007197ca5582f0adf8a/index.js#L102

This initial solution prints out the details of all entries in .errors, so that the detailed/formatted original errors are visible in the console.

[n1ru4l]

@jlmessenger Instead of doing this in cli-error.ts, can we apply this formatting in executeCodegen?

graphql-code-generator/packages/graphql-codegen-cli/src/codegen.ts

Line 36 in 39773f5

export async function executeCodegen(input: CodegenContext | Types.Config): Promise<Types.FileOutput[]> {

That way also the programmatic usage will yield useable error messages.

Also, we need failing tests for progressing this further.

@jlmessenger Can you please continue with that?

[jlmessenger]

Hi @n1ru4l - I want to ensure I understand your preferred approach before I rework the commit.

• Capture the listr thrown error within executeCodegen
• Extract the error details and unpack the attached errors into a DetailedError
• Thus, all downstream code can stay unaware of listr errors?

Draft Proposal:

Update the following area of executeCodegen

graphql-code-generator/packages/graphql-codegen-cli/src/codegen.ts

Line 321 in 39773f5

await listr.run();

try {
  await listr.run();
} catch (err) {
  if (isListrError(err)) {
    if (err.errors.length === 1) {
      throw err.errors[0]!; // only one error, so re-throw it
    }
    // Multiple errors, so unpack into a single DetailedError
    const allErrs = err.errors.map(subErr => isDetailedError(errItem)
      ? `${subErr.message} for "${subErr.source}"${subErr.details}`
      : subErr.message || subErr.toString()
    );
    const newErr = new DetailedError(`${err.message}${allErrs.join("\n\n")}`);
    // Best-effort to all stack traces for debugging
    newErr.stack = `${newErr.stack}\n\n${err.errors.map(subErr => subErr.stack).join("\n\n")}`;
    throw newErr;
  }
  throw err;
}

The above example assumes a type-guard function to detect and cast listr errors:

type CompositeError = Error | DetailedError;
type ListrError = Error & { errors: CompositeError[] };
function isListrError(err: Error & { name?: unknown, errors?: unknown }): err is ListrError {
  return err.name === "ListrError" && Array.isArray(err.errors) && err.errors.length > 0;
}

[n1ru4l]

@jlmessenger Hey, sorry for the late reply 😓

yes, this is exactly what we should do!

[s0ber]

It will be very nice to have this fix! Validation is almost unusable in a context of monorepo right now. The error output is swallowed both by lerna and yarn2 workspaces. Will try to test it locally and will let you know if it'll help with lerna (today or tomorrow).

[s0ber]

@jlmessenger @n1ru4l Just want to let you know that the PR patch is astonishing, amazing, mesmerizing, incredible, brilliant and completely perfect. And it solved our issue with lerna 👍

[xanderdeseyn]

Will this PR be merged? Would help our team a lot!

[ardatan]

Sorry for late response. Could you add a changeset with yarn changeset?
And do you think if it is possible to have unit tests for this change so we can make sure we won't break it in the future?

[n1ru4l]

This PR should be adjusted as discribed in #6921 (comment)

[charlypoly]

@n1ru4l I did the requested changes, however, I feel like the errors manipulation should be done at the print level ( packages/graphql-codegen-cli/src/cli.ts), in order to keep original errors thrown by executeCodeGen()

WDYT?