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

bootstrap: usable error on missing internal module #29593

Conversation

Fishrock123
Copy link
Contributor

Due to how bootstrap/loaders.js itself is loaded and invoked, stacktraces from it are munged and no longer point back to the error source.

That resulted in the following unhelpful error if an internal module was missing or misnamed:

internal/bootstrap/loaders.js:190
  return mod.compile();
             ^

TypeError: Cannot read property 'compile' of undefined

This changes that to at least print the id that was attempted to be loaded:

internal/bootstrap/loaders.js:189
  if (!mod) throw new TypeError(`Missing internal module '${id}'`);
            ^

TypeError: Missing internal module 'internal/a'
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@Fishrock123 Fishrock123 added the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 17, 2019
@addaleax
Copy link
Member

I’m okay with fast-tracking this if you like.

@addaleax addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Sep 17, 2019
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Sep 17, 2019
@Fishrock123
Copy link
Contributor Author

Hmm, guess I forgot to run tests / lint last minute? (I swear I did...)

/home/travis/build/nodejs/node/lib/internal/bootstrap/loaders.js
189:19 error Use an error exported by the internal/errors module no-restricted-syntax
✖ 1 problem (1 error, 0 warnings)

Not exactly useful here, what's the best way to exclude this rule from this file entirely?

@devsnek
Copy link
Member

devsnek commented Sep 17, 2019

i'd just // eslint-disable-next-line no-restricted-syntax

@Fishrock123 Fishrock123 force-pushed the internal-module-load-error-message branch from add9401 to 17df5b6 Compare September 17, 2019 22:47
Due to how bootstrap/loaders.js itself is loaded and invoked,
stacktraces from it are munged and no longer point back to the error
source.

That resulted in the following unhelpful error if an internal module
was missing or misnamed:

```
internal/bootstrap/loaders.js:190
  return mod.compile();
             ^

TypeError: Cannot read property 'compile' of undefined
```

This changes that to at least print the id that was attempted to be
loaded:

```
internal/bootstrap/loaders.js:189
  if (!mod) throw new TypeError(`Missing internal module '${id}'`);
            ^

TypeError: Missing internal module 'internal/a'
```
@Fishrock123 Fishrock123 force-pushed the internal-module-load-error-message branch from 17df5b6 to 9f804d1 Compare September 17, 2019 23:01
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@Trott
Copy link
Member

Trott commented Sep 18, 2019

Landed in 4d798e1

@Trott Trott closed this Sep 18, 2019
Trott pushed a commit that referenced this pull request Sep 18, 2019
Due to how bootstrap/loaders.js itself is loaded and invoked,
stacktraces from it are munged and no longer point back to the error
source.

That resulted in the following unhelpful error if an internal module
was missing or misnamed:

```
internal/bootstrap/loaders.js:190
  return mod.compile();
             ^

TypeError: Cannot read property 'compile' of undefined
```

This changes that to at least print the id that was attempted to be
loaded:

```
internal/bootstrap/loaders.js:189
  if (!mod) throw new TypeError(`Missing internal module '${id}'`);
            ^

TypeError: Missing internal module 'internal/a'
```

PR-URL: #29593
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@Fishrock123
Copy link
Contributor Author

(I actually think TypeError was the wrong choice, but ok, it probably doesn’t matter..)

@joyeecheung
Copy link
Member

This only works specifically for the errors caused by incorrect internal module ids (however throwing here right after failure of lookup with the module id in the message still SGTM). #29624 should help printing stack traces for all kinds of errors thrown during bootstrap.

targos pushed a commit that referenced this pull request Sep 20, 2019
Due to how bootstrap/loaders.js itself is loaded and invoked,
stacktraces from it are munged and no longer point back to the error
source.

That resulted in the following unhelpful error if an internal module
was missing or misnamed:

```
internal/bootstrap/loaders.js:190
  return mod.compile();
             ^

TypeError: Cannot read property 'compile' of undefined
```

This changes that to at least print the id that was attempted to be
loaded:

```
internal/bootstrap/loaders.js:189
  if (!mod) throw new TypeError(`Missing internal module '${id}'`);
            ^

TypeError: Missing internal module 'internal/a'
```

PR-URL: #29593
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@BridgeAR BridgeAR mentioned this pull request Sep 24, 2019
Trott pushed a commit to Trott/io.js that referenced this pull request Sep 25, 2019
So that the stack trace of errors shown in internal code run during
bootstrap (before process._fatalException is set) can be printed.

PR-URL: nodejs#29624
Refs: nodejs#29593
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
BridgeAR pushed a commit that referenced this pull request Sep 25, 2019
Due to how bootstrap/loaders.js itself is loaded and invoked,
stacktraces from it are munged and no longer point back to the error
source.

That resulted in the following unhelpful error if an internal module
was missing or misnamed:

```
internal/bootstrap/loaders.js:190
  return mod.compile();
             ^

TypeError: Cannot read property 'compile' of undefined
```

This changes that to at least print the id that was attempted to be
loaded:

```
internal/bootstrap/loaders.js:189
  if (!mod) throw new TypeError(`Missing internal module '${id}'`);
            ^

TypeError: Missing internal module 'internal/a'
```

PR-URL: #29593
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
targos pushed a commit that referenced this pull request Oct 1, 2019
So that the stack trace of errors shown in internal code run during
bootstrap (before process._fatalException is set) can be printed.

PR-URL: #29624
Refs: #29593
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. fast-track PRs that do not need to wait for 48 hours to land. lib / src Issues and PRs related to general changes in the lib or src directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants