FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

bootstrap: usable error on missing internal module by Fishrock123 · Pull Request #29593 · nodejs/node · GitHub

/ node Public

bootstrap: usable error on missing internal module - #29593

Closed
Fishrock123 wants to merge 1 commit into
nodejs:masterfrom
Fishrock123:internal-module-load-error-message
ConversationCommits (1)ChecksFiles changed
Closed

bootstrap: usable error on missing internal module#29593
Fishrock123 wants to merge 1 commit into
nodejs:masterfrom
Fishrock123:internal-module-load-error-message

Conversation

Fishrock123 commented Sep 17, 2019

Copy link
Copy Markdown
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 added the lib / src Issues and PRs related to general changes in the lib or src directory. label Sep 17, 2019

addaleax commented Sep 17, 2019

Copy link
Copy Markdown
Member

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

addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Sep 17, 2019

nodejs-github-bot commented Sep 17, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25521/

addaleax added the author ready PRs that have at least one approval, no outstanding review comments, and a CI started. label Sep 17, 2019

Fishrock123 commented Sep 17, 2019

Copy link
Copy Markdown
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 commented Sep 17, 2019

Copy link
Copy Markdown
Member

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

Fishrock123 force-pushed the internal-module-load-error-message branch from add9401 to 17df5b6 Compare September 17, 2019 22:47
bootstrap: usable error on missing internal module
9f804d1 
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 force-pushed the internal-module-load-error-message branch from 17df5b6 to 9f804d1 Compare September 17, 2019 23:01

nodejs-github-bot commented Sep 17, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25529/

nodejs-github-bot commented Sep 18, 2019

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/25534/

Trott commented Sep 18, 2019

Copy link
Copy Markdown
Member

Landed in 4d798e1

Trott closed this Sep 18, 2019
Trott pushed a commit that referenced this pull request Sep 18, 2019
bootstrap: provide usable error on missing internal module
4d798e1 
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 commented Sep 18, 2019

Copy link
Copy Markdown
Contributor Author

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

joyeecheung mentioned this pull request Sep 20, 2019
Closed
4 tasks

joyeecheung commented Sep 20, 2019

Copy link
Copy Markdown
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
bootstrap: provide usable error on missing internal module
c28c955 
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 mentioned this pull request Sep 24, 2019
Merged
Trott pushed a commit to Trott/io.js that referenced this pull request Sep 25, 2019
src: try showing stack traces when process._fatalException is not set
355f2ad 
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
bootstrap: provide usable error on missing internal module
c052967 
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>
This was referenced Sep 25, 2019
Closed
Closed
This was referenced Sep 26, 2019
Closed
Closed
Closed
Closed
targos pushed a commit that referenced this pull request Oct 1, 2019
src: try showing stack traces when process._fatalException is not set
8d88010 
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

jasnell jasnell approved these changes
addaleax addaleax approved these changes
cjihrig cjihrig approved these changes
BridgeAR BridgeAR approved these changes
joyeecheung Awaiting requested review from joyeecheung
+1 more reviewer
devsnek devsnek approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

author ready PRs that have at least one approval, no outstanding review comments, 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

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL