Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error from `loadPackage` swallowed in node 0.10 and 0.12 #4691

Closed
terinjokes opened this Issue Dec 14, 2015 · 15 comments

Comments

Projects
None yet
4 participants
@terinjokes
Copy link

terinjokes commented Dec 14, 2015

While debugging why Travis CI was running into failing builds only with Node.js 4.2 for my ESLint plugin, I've noticed the errors from lib/config/config-file.js's loadPackage function are swallowed in Node.js 0.10 and 0.12.

cloudflare/eslint-plugin-cflint Travis CI run

In Node.js 4.2 the following error is reported with a non-zero exit code:

Error: Cannot read config package: eslint-config-xo
Error: Cannot find module 'eslint-config-xo'
Referenced from: 
    at Function.Module._resolveFilename (module.js:337:15)
    at Function.Module._load (module.js:287:25)
    at Module.require (module.js:366:17)
    at require (module.js:385:17)
    at loadPackage (/home/travis/build/cloudflare/eslint-plugin-cflint/node_modules/eslint/lib/config/config-file.js:168:16)
    at loadConfigFile (/home/travis/build/cloudflare/eslint-plugin-cflint/node_modules/eslint/lib/config/config-file.js:212:18)
    at load (/home/travis/build/cloudflare/eslint-plugin-cflint/node_modules/eslint/lib/config/config-file.js:385:18)
    at /home/travis/build/cloudflare/eslint-plugin-cflint/node_modules/eslint/lib/config/config-file.js:326:36
    at Array.reduceRight (native)
    at Object.applyExtends (/home/travis/build/cloudflare/eslint-plugin-cflint/node_modules/eslint/lib/config/config-file.js:309:28)

Despite having the same tree, 0.10 and 0.12 do not print a warning and exit successfully. This is a lie.

If you run with the DEBUG environment variable set, you see that the debug mess from line 170 runs.

  eslint:config-file Loading xo +0ms
  eslint:config-file Loading config package: eslint-config-xo +1ms
  eslint:config-file Error reading package: eslint-config-xo +5ms

@eslintbot eslintbot added the triage label Dec 14, 2015

@eslintbot

This comment has been minimized.

Copy link

eslintbot commented Dec 14, 2015

@terinjokes Thanks for the issue! If you're reporting a bug, please be sure to include:

  1. The version of ESLint you are using (run eslint -v)
  2. What you did (the source code and ESLint configuration)
  3. The actual ESLint output complete with numbers
  4. What you expected to happen instead

Requesting a new rule? Please see Proposing a New Rule for instructions.

@ilyavolodin ilyavolodin added bug core evaluating and removed triage labels Dec 14, 2015

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Dec 14, 2015

Hmm.. interesting. What's the version of ESLint you are using?

@terinjokes

This comment has been minimized.

Copy link
Author

terinjokes commented Dec 14, 2015

eslint@1.10.3.

Worth noting that this ticket isn't about the error message itself, just
the fact that it's missing.
On Dec 14, 2015 7:43 AM, "Ilya Volodin" notifications@github.com wrote:

Hmm.. interesting. What's the version of ESLint you are using?


Reply to this email directly or view it on GitHub
#4691 (comment).

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 14, 2015

Have you experienced this when running locally, or just with Travis? If the latter, can you try running locally?

@terinjokes

This comment has been minimized.

Copy link
Author

terinjokes commented Dec 14, 2015

I'm able to reproduce this locally. The output with DEBUG=* is from my local system.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 14, 2015

Hmm, I'm not able to reproduce this. I'm using Node v0.10.40 and I see the error being thrown and the exit code is 8. I also tried on v0.10.41 and got the same.

Can anyone else verify this on Node 0.10 or 0.12?

@terinjokes

This comment has been minimized.

Copy link
Author

terinjokes commented Dec 14, 2015

I think @sindresorhus was able to reproduce when I asked if he'd seen it before (and xo being his package)

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 15, 2015

Sorry, was looking for responses from the team, basically someone who can reproduce and investigate.

@terinjokes

This comment has been minimized.

Copy link
Author

terinjokes commented Dec 15, 2015

Oh, sorry.

I'll upload a small case that fails here in a few minutes.
On Dec 15, 2015 9:19 AM, "Nicholas C. Zakas" notifications@github.com
wrote:

Sorry, was looking for responses from the team, basically someone who can
reproduce and investigate.


Reply to this email directly or view it on GitHub
#4691 (comment).

@terinjokes

This comment has been minimized.

Copy link
Author

terinjokes commented Dec 15, 2015

@nzakas & team:

I've pushed a repository where I can reproduce the issue for 0.12. https://github.com/terinjokes/eslint-silent-error.

I've not yet determined why I'm not able to reproduce the 0.10 issue.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 15, 2015

My VM got corrupted thanks to a power failure. As soon as I set it back up, I'll check your example.

@ilyavolodin

This comment has been minimized.

Copy link
Member

ilyavolodin commented Dec 16, 2015

I was able to reproduce an issue with Node v0.12.7 but not with v0.10. I'm not 100% sure that's ESLint's issue. It feels like it's something off with Node itself.

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Dec 16, 2015

I'm sick in bed today, but I wonder if this could be the issue: https://github.com/eslint/eslint/blob/master/bin/eslint.js#L67

If 0.12 changed his process exiting worked at all, this could be a race condition we are losing.

@terinjokes

This comment has been minimized.

Copy link
Author

terinjokes commented Dec 16, 2015

@nzakas 0.12 is when process.exitCode was added. For 0.10 I've used exit-code as a shim (which ends conditionally doing the same logic)

@nzakas

This comment has been minimized.

Copy link
Member

nzakas commented Jan 6, 2016

Working on this

@nzakas nzakas closed this in 688f277 Jan 6, 2016

ilyavolodin added a commit that referenced this issue Jan 6, 2016

Merge pull request #4863 from eslint/issue4691
Fix: Set proper exit code for Node > 0.10 (fixes #4691)

@eslint eslint bot locked and limited conversation to collaborators Feb 6, 2018

@eslint eslint bot added the archived due to age label Feb 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.