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.

Sign up for GitHub

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

Jump to bottom

src: throw error in LoadBuiltinModuleSource when reading fails #38904

Closed
wants to merge 4 commits into from
Closed

src: throw error in LoadBuiltinModuleSource when reading fails #38904

wants to merge 4 commits into from

Conversation

joyeecheung
Copy link
Member

  • Move the file reading code in LoadBuiltinModuleSource into
    util.h so that it can be reused by other C++ code, and
    return an error code from it when there is a failure for
    the caller to generate an error.
  • Throw an error when reading local builtins fails in
    LoadBulitinModuleSource.

The error looks something like this

node:internal/bootstrap/loaders:311
      const fn = compileFunction(id);
                 ^

Error: Cannot read local builtin. ENOENT: no such file or directory "/Users/joyee/projects/node/lib/fs.js"
    at NativeModule.compileForInternalLoader (node:internal/bootstrap/loaders:311:18)
    at nativeModuleRequire (node:internal/bootstrap/loaders:341:14)
    at node:internal/bootstrap/node:362:1

@joyeecheung
src: throw error in LoadBuiltinModuleSource when reading fails
c557439 
- Move the file reading code in LoadBuiltinModuleSource into
  util.h so that it can be reused by other C++ code, and
  return an error code from it when there is a failure for
  the caller to generate an error.
- Throw an error when reading local builtins fails in
  LoadBulitinModuleSource.
@github-actions github-actions bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Jun 2, 2021
@nodejs-github-bot
Copy link
Collaborator

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

bl-ue
bl-ue approved these changes Jun 2, 2021
View reviewed changes
src/util.h Outdated Show resolved Hide resolved
RaisinTen
RaisinTen reviewed Jun 2, 2021
View reviewed changes
src/node_native_module.cc Outdated Show resolved Hide resolved
joyeecheung and others added 2 commits June 2, 2021 21:16
@joyeecheung @bl-ue
fixup! src: throw error in LoadBuiltinModuleSource when reading fails
e0b2b5a 
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
@joyeecheung @RaisinTen
fixup! src: throw error in LoadBuiltinModuleSource when reading fails
d88d33d 
Co-authored-by: Darshan Sen <raisinten@gmail.com>
@nodejs-github-bot
Copy link
Collaborator

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

@devsnek
Copy link
Member

devsnek commented Jun 2, 2021

while you're at it you could also replace the other file reading stuff, like what #31844 has, though that could be separate changes too.

@nodejs-github-bot
Copy link
Collaborator

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

@joyeecheung joyeecheung mentioned this pull request Jun 2, 2021
Closed
@joyeecheung
Copy link
Member Author

cc @nodejs/startup can I have some review please?

@joyeecheung joyeecheung added the review wanted PRs that need reviews. label Jun 7, 2021
legendecas
legendecas reviewed Jun 8, 2021
View reviewed changes
src/util.cc Show resolved Hide resolved
@nodejs-github-bot

This comment has been minimized.

Sign in to view
@nodejs-github-bot
Copy link
Collaborator

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

joyeecheung added a commit that referenced this pull request Jun 10, 2021
@joyeecheung
src: throw error in LoadBuiltinModuleSource when reading fails
8834ec9 
- Move the file reading code in LoadBuiltinModuleSource into
  util.h so that it can be reused by other C++ code, and
  return an error code from it when there is a failure for
  the caller to generate an error.
- Throw an error when reading local builtins fails in
  LoadBulitinModuleSource.

PR-URL: #38904
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@joyeecheung
Copy link
Member Author

Landed in 8834ec9

targos pushed a commit that referenced this pull request Jun 11, 2021
@joyeecheung @targos
src: throw error in LoadBuiltinModuleSource when reading fails
ff8313c 
- Move the file reading code in LoadBuiltinModuleSource into
  util.h so that it can be reused by other C++ code, and
  return an error code from it when there is a failure for
  the caller to generate an error.
- Throw an error when reading local builtins fails in
  LoadBulitinModuleSource.

PR-URL: #38904
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@danielleadams danielleadams mentioned this pull request Jun 14, 2021
Merged
danielleadams pushed a commit that referenced this pull request Jun 17, 2021
@joyeecheung @danielleadams
src: throw error in LoadBuiltinModuleSource when reading fails
2d7acd0 
- Move the file reading code in LoadBuiltinModuleSource into
  util.h so that it can be reused by other C++ code, and
  return an error code from it when there is a failure for
  the caller to generate an error.
- Throw an error when reading local builtins fails in
  LoadBulitinModuleSource.

PR-URL: #38904
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 19, 2021
@joyeecheung @richardlau
src: throw error in LoadBuiltinModuleSource when reading fails
1c1b387 
- Move the file reading code in LoadBuiltinModuleSource into
  util.h so that it can be reused by other C++ code, and
  return an error code from it when there is a failure for
  the caller to generate an error.
- Throw an error when reading local builtins fails in
  LoadBulitinModuleSource.

PR-URL: #38904
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
richardlau pushed a commit that referenced this pull request Jul 20, 2021
@joyeecheung @richardlau
src: throw error in LoadBuiltinModuleSource when reading fails
bee477b 
- Move the file reading code in LoadBuiltinModuleSource into
  util.h so that it can be reused by other C++ code, and
  return an error code from it when there is a failure for
  the caller to generate an error.
- Throw an error when reading local builtins fails in
  LoadBulitinModuleSource.

PR-URL: #38904
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
@richardlau richardlau mentioned this pull request Jul 20, 2021
Merged
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
@joyeecheung @foxxyz
src: throw error in LoadBuiltinModuleSource when reading fails
5540620 
- Move the file reading code in LoadBuiltinModuleSource into
  util.h so that it can be reused by other C++ code, and
  return an error code from it when there is a failure for
  the caller to generate an error.
- Throw an error when reading local builtins fails in
  LoadBulitinModuleSource.

PR-URL: nodejs#38904
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@RaisinTen RaisinTen RaisinTen left review comments

@legendecas legendecas legendecas approved these changes

@bl-ue bl-ue bl-ue approved these changes

Assignees
No one assigned
Labels
c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. review wanted PRs that need reviews.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@joyeecheung @nodejs-github-bot @devsnek @legendecas @RaisinTen @bl-ue