Bug: Duplicate error message displayed when flat config contains eslintrc top-level key

[nzakas]

Environment

Node version: v18.16.0
npm version: v7.10.0
Local ESLint version: v8.49.0 (Currently used)
Global ESLint version: Not found
Operating System: win32 10.0.19045

What parser are you using?

Default (Espree)

What did you do?

Created an eslint.config.js file with globals as a top-level key.

Configuration

export default [
    {
        globals: {},
    }
];

let x = 1;

What did you expect to happen?

I should get an error message explaining that globals is invalid.

What actually happened?

I got two error messages explaining that globals is invalid.

$ npm run lint

> @humanwhocodes/env@3.0.0 lint
> eslint src/ tests/


Oops! Something went wrong! :(

ESLint: 8.47.0

A config object is using the "globals" key, which is not supported in flat config system.

Flat config uses "languageOptions.globals" to define global variables for your files.

Please see the following page for information on how to convert your config object into the correct format:   
https://eslint.org/docs/latest/use/configure/migration-guide#configuring-language-options

Oops! Something went wrong! :(

ESLint: 8.47.0

A config object is using the "globals" key, which is not supported in flat config system.

Flat config uses "languageOptions.globals" to define global variables for your files.

Please see the following page for information on how to convert your config object into the correct format:   
https://eslint.org/docs/latest/use/configure/migration-guide#configuring-language-options

Link to Minimal Reproducible Example

https://stackblitz.com/edit/stackblitz-starters-yk4rv3

Participation

• I am willing to submit a pull request for this issue.

Additional comments

Interestingly, the StackBlitz repro doesn't show this behavior but I'm able to reproduce it consistently locally (on Windows in VS Code).

[Rec0iL99]

I'm on macOS and I couldn't reproduce this locally with VSCode.

> npx eslint index.js                                                                                                                              

Oops! Something went wrong! :(

ESLint: 8.49.0

A config object is using the "globals" key, which is not supported in flat config system.

Flat config uses "languageOptions.globals" to define global variables for your files.

Please see the following page for information on how to convert your config object into the correct format:
https://eslint.org/docs/latest/use/configure/migration-guide#configuring-language-options

Could this be a Windows specific issue?

[nzakas]

Could this be a Windows specific issue?

That's what I'm wondering. Need someone else with Windows to confirm.

[fasttime]

I can reproduce this on Windows and MacOS if I pass two non-empty directories as command-line arguments (or the same argument twice):

npx eslint src/ test/

My repro: https://stackblitz.com/edit/stackblitz-starters-tqtbuu?file=README.md

Not sure why the behavior in StackBlitz is different.

[mdjermanovic]

My repro: https://stackblitz.com/edit/stackblitz-starters-tqtbuu?file=README.md

I can reproduce this on Windows.

I'd guess this has something to do with how @nodelib/fs.walk manages parallel work. When I add concurrency: 1, as an option to doFsWalk here, I get only 1 error.

[mdjermanovic]

uncaughtException is emitted twice, with:

Error: Key "globals": This appears to be in eslintrc format rather than flat config format.
    at Object.validate (C:\milos\tmp\tmp\node_modules\eslint\lib\config\flat-config-schema.js:488:19)
    at ObjectSchema.validate (C:\milos\tmp\tmp\node_modules\@humanwhocodes\object-schema\src\object-schema.js:218:35)
    at C:\milos\tmp\tmp\node_modules\@humanwhocodes\object-schema\src\object-schema.js:171:18
    at Array.reduce (<anonymous>)
    at ObjectSchema.merge (C:\milos\tmp\tmp\node_modules\@humanwhocodes\object-schema\src\object-schema.js:169:24)
    at C:\milos\tmp\tmp\node_modules\@humanwhocodes\config-array\api.js:966:42
    at Array.reduce (<anonymous>)
    at FlatConfigArray.getConfig (C:\milos\tmp\tmp\node_modules\@humanwhocodes\config-array\api.js:965:39)
    at FlatConfigArray.isFileIgnored (C:\milos\tmp\tmp\node_modules\@humanwhocodes\config-array\api.js:993:15)
    at C:\milos\tmp\tmp\node_modules\eslint\lib\eslint\eslint-helpers.js:313:49
    at Array.reduce (<anonymous>)
    at entryFilter (C:\milos\tmp\tmp\node_modules\eslint\lib\eslint\eslint-helpers.js:300:28)
    at Object.isAppliedFilter (C:\milos\tmp\tmp\node_modules\@nodelib\fs.walk\out\readers\common.js:12:31)
    at AsyncReader._handleEntry (C:\milos\tmp\tmp\node_modules\@nodelib\fs.walk\out\readers\async.js:86:20)
    at C:\milos\tmp\tmp\node_modules\@nodelib\fs.walk\out\readers\async.js:65:22
    at callSuccessCallback (C:\milos\tmp\tmp\node_modules\@nodelib\fs.scandir\out\providers\async.js:103:5)
    at C:\milos\tmp\tmp\node_modules\@nodelib\fs.scandir\out\providers\async.js:29:13
    at node:fs:188:23
    at node:internal/util:443:5
    at getDirents (node:internal/fs/utils:275:7)
    at FSReqCallback.req.oncomplete (node:fs:1369:7)

Perhaps we should do process.exit(2) in function onFatalError, instead of just setting process.exitCode = 2?

[mdjermanovic]

Perhaps we should do process.exit(2) in function onFatalError, instead of just setting process.exitCode = 2?

Relevant: #7569.

[fasttime]

Perhaps we should do process.exit(2) in function onFatalError, instead of just setting process.exitCode = 2?

Relevant: #7569.

process.exit() is still problematic because it terminates the process without flushing the console output. Another option would be to ignore unhandled errors after the first one.

[nzakas]

Maybe we could change process.on() to process.once() here:

eslint/bin/eslint.js

Lines 119 to 120 in 55c1685

	process.on("uncaughtException", onFatalError);
	process.on("unhandledRejection", onFatalError);

?

[fasttime]

Maybe we could change process.on() to process.once() here:

eslint/bin/eslint.js

Lines 119 to 120 in 55c1685

	process.on("uncaughtException", onFatalError);
	process.on("unhandledRejection", onFatalError);

?

That would causes Node.js to print a stacktrace of the second error, because it's unhandled.

This is what the output on MacOS looks like when process.on is replaced with process.once:

> lint
> eslint src/ test/


Oops! Something went wrong! :(

ESLint: 8.47.0

A config object is using the "globals" key, which is not supported in flat config system.

Flat config uses "languageOptions.globals" to define global variables for your files.

Please see the following page for information on how to convert your config object into the correct format:
https://eslint.org/docs/latest/use/configure/migration-guide#configuring-language-options
[...]/node_modules/@humanwhocodes/object-schema/src/object-schema.js:221
                throw ex;
                ^

IncompatibleKeyError: Key "globals": This appears to be in eslintrc format rather than flat config format.
    at Object.validate ([...]/node_modules/eslint/lib/config/flat-config-schema.js:488:19)
    at ObjectSchema.validate ([...]/node_modules/@humanwhocodes/object-schema/src/object-schema.js:218:35)
    at [...]/node_modules/@humanwhocodes/object-schema/src/object-schema.js:171:18
    at Array.reduce (<anonymous>)
    at ObjectSchema.merge ([...]/node_modules/@humanwhocodes/object-schema/src/object-schema.js:169:24)
    at [...]/node_modules/@humanwhocodes/config-array/api.js:966:42
    at Array.reduce (<anonymous>)
    at FlatConfigArray.getConfig ([...]/node_modules/@humanwhocodes/config-array/api.js:965:39)
    at FlatConfigArray.isFileIgnored ([...]/node_modules/@humanwhocodes/config-array/api.js:993:15)
    at [...]/node_modules/eslint/lib/eslint/eslint-helpers.js:312:49 {
  messageTemplate: 'eslintrc-incompat',
  messageData: { key: 'globals' }
}

Node.js v20.6.1

Not sure where the last line with "Node.js v20.6.1" is coming from.

[mdjermanovic]

We could update function onFatalError to log an error only on the first call. That should fix the problem of duplicate error messages, but I'm still not sure if continuing with execution after an uncaught error is a good practice (though, I don't know how we could make sure that the console output is flushed if we do process.exit). When the #7569 change was made, eslint was entirely sync I believe, so this wasn't an issue.

[nzakas]

I'm still not sure if continuing with execution after an uncaught error is a good practice

Execution isn't actually continuing here , it's already stopped. onFatalError is called as the last stop before the process exits. That there's another thread that will produce the same error is ultimately inconsequential because it also does not continue.

All that is to say that ensuring a duplicate message isn't output is probably the path of least resistance to addressing this and I don't think there are any side effects to be concerned with.

[mdjermanovic]

In some cases, eslint could keep working for a long time after an error occurs.

For example, add this to create(context) of the no-undef rule:

if (context.filename.endsWith("Makefile.js")) {
    throw new Error("Crashed!");
}

Then run npx eslint .

The error appears quickly because Makefile.js is one of the first files that's linted:

Oops! Something went wrong! :(

ESLint: 8.49.0

Error: Error while loading rule 'no-undef': Crashed!
Occurred while linting C:\milos\eslint\Makefile.js
    at Object.create (C:\milos\eslint\lib\rules\no-undef.js:56:19)
    at createRuleListeners (C:\milos\eslint\lib\linter\linter.js:870:21)
    at C:\milos\eslint\lib\linter\linter.js:1041:110
    at Array.forEach (<anonymous>)
    at runRules (C:\milos\eslint\lib\linter\linter.js:978:34)
    at Linter._verifyWithFlatConfigArrayAndWithoutProcessors (C:\milos\eslint\lib\linter\linter.js:1631:31)
    at Linter._verifyWithFlatConfigArray (C:\milos\eslint\lib\linter\linter.js:1761:21)
    at Linter.verify (C:\milos\eslint\lib\linter\linter.js:1409:65)
    at Linter.verifyAndFix (C:\milos\eslint\lib\linter\linter.js:1968:29)
    at verifyText (C:\milos\eslint\lib\eslint\flat-eslint.js:475:48)

But then it continues linting other files, which seems wrong because the rule and its callers (e.g., linter) are possibly in a broken state, and it takes 30-40s before the command prompt appears again.

[nzakas]

@mdjermanovic that seems like a bug in error handling for rules that should be fixed? Do you want to open a separate issue for that?

For some further context: testing locally it seems that running ESLint via npm run results in two copies of the message while running directly via npx eslint results in a single error message.

I haven't been able to come up with an automated test that displays this behavior, but I did put together a PR with a possible approach: #17584

[mdjermanovic]

that seems like a bug in error handling for rules that should be fixed? Do you want to open a separate issue for that?

I thought we would fix this by exiting the process, and that would fix the problem I mentioned as well (though only when ESLint is used through CLI).

I opened a separate issue now: #17621