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

errors: use ErrorPrototypeToString from primordials object #34891

Conversation

ExE-Boss
Copy link
Contributor

This ensures that the prepareStackTrace no override fallback path doesn’t get broken by Error.prototype.toString.call not being Function.prototype.call.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Aug 23, 2020
@nodejs-github-bot
Copy link
Collaborator

@devsnek devsnek added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 25, 2020
@github-actions github-actions bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Aug 25, 2020
@github-actions
Copy link
Contributor

Commit Queue failed
- Loading data for nodejs/node/pull/34891
✔  Done loading data for nodejs/node/pull/34891
----------------------------------- PR info ------------------------------------
Title      errors: Use `ErrorPrototypeToString` from `primordials` object (#34891)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     ExE-Boss:lib/internal/errors/use-primordials-error-prototype-tostring -> nodejs:master
Labels     errors
Commits    1
 - errors: Use `ErrorPrototypeToString` from `primordials` object
Committers 1
 - ExE Boss <3889017+ExE-Boss@users.noreply.github.com>
PR-URL: https://github.com/nodejs/node/pull/34891
Reviewed-By: Gus Caplan 
Reviewed-By: Michaël Zasso 
Reviewed-By: Ruben Bridgewater 
Reviewed-By: James M Snell 
Reviewed-By: Luigi Pinca 
Reviewed-By: Colin Ihrig 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/34891
Reviewed-By: Gus Caplan 
Reviewed-By: Michaël Zasso 
Reviewed-By: Ruben Bridgewater 
Reviewed-By: James M Snell 
Reviewed-By: Luigi Pinca 
Reviewed-By: Colin Ihrig 
--------------------------------------------------------------------------------
   ℹ  Last Full PR CI on 2020-08-23T17:41:17Z: https://ci.nodejs.org/job/node-test-pull-request/32907/
- Querying data of job/node-test-pull-request/32907/
✔  Build data downloaded
   ℹ  This PR was created on Sun, 23 Aug 2020 15:45:55 GMT
   ✔  Approvals: 6
   ✔  - Gus Caplan (@devsnek): https://github.com/nodejs/node/pull/34891#pullrequestreview-473024249
   ✔  - Michaël Zasso (@targos) (TSC): https://github.com/nodejs/node/pull/34891#pullrequestreview-473024251
   ✔  - Ruben Bridgewater (@BridgeAR) (TSC): https://github.com/nodejs/node/pull/34891#pullrequestreview-473026575
   ✔  - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/34891#pullrequestreview-473596914
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/34891#pullrequestreview-473794806
   ✔  - Colin Ihrig (@cjihrig) (TSC): https://github.com/nodejs/node/pull/34891#pullrequestreview-474025385
--------------------------------------------------------------------------------
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/master up to date...
From https://github.com/nodejs/node
 * branch              master     -> FETCH_HEAD
✔  origin/master is now up-to-date
- Downloading patch for 34891
✔  Downloaded patch to /home/runner/work/node/node/.ncu/34891/patch
--------------------------------------------------------------------------------
Applying: errors: Use `ErrorPrototypeToString` from `primordials` object
   ✔  Patches applied
--------------------------------------------------------------------------------
--------------------------------- New Message ----------------------------------
errors: Use `ErrorPrototypeToString` from `primordials` object

PR-URL: #34891
Reviewed-By: Gus Caplan me@gus.host
Reviewed-By: Michaël Zasso targos@protonmail.com
Reviewed-By: Ruben Bridgewater ruben@bridgewater.de
Reviewed-By: James M Snell jasnell@gmail.com
Reviewed-By: Luigi Pinca luigipinca@gmail.com
Reviewed-By: Colin Ihrig cjihrig@gmail.com

[master 7235e929] errors: Use ErrorPrototypeToString from primordials object
Author: ExE Boss 3889017+ExE-Boss@users.noreply.github.com
Date: Sun Aug 23 17:40:00 2020 +0200
1 file changed, 2 insertions(+), 2 deletions(-)
✖ 7235e92928995d37b42429997e6b785de572805b
✔ 0:0 skipping fixes-url fixes-url
✔ 0:0 blank line after title line-after-title
✔ 0:0 line-lengths are valid line-length
✔ 0:0 metadata is at end of message metadata-end
✔ 1:8 PR-URL is valid. pr-url
✔ 0:0 reviewers are valid reviewers
✔ 0:0 valid subsystems subsystem
✖ 0:9 First word after subsystem(s) in title should be lowercase. title-format
⚠ 0:50 Title should be <= 50 columns. title-length

ℹ Please fix the commit message and try again.

@ExE-Boss ExE-Boss changed the title errors: Use ErrorPrototypeToString from primordials object errors: use ErrorPrototypeToString from primordials object Aug 25, 2020
@ExE-Boss ExE-Boss force-pushed the lib/internal/errors/use-primordials-error-prototype-tostring branch from c38afbb to 1df8048 Compare August 25, 2020 22:31
Copy link
Contributor

@cjihrig cjihrig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM if the commit queue bot is OK with the commit message.

@devsnek devsnek added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Aug 26, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 26, 2020
@nodejs-github-bot
Copy link
Collaborator

@devsnek devsnek added the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 26, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 26, 2020
@github-actions
Copy link
Contributor

Landed in 565d993

@github-actions github-actions bot closed this Aug 26, 2020
nodejs-github-bot pushed a commit that referenced this pull request Aug 26, 2020
PR-URL: #34891
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@ExE-Boss ExE-Boss deleted the lib/internal/errors/use-primordials-error-prototype-tostring branch August 26, 2020 14:01
@ExE-Boss
Copy link
Contributor Author

This should probably also be backported.

richardlau pushed a commit that referenced this pull request Sep 1, 2020
PR-URL: #34891
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@richardlau richardlau mentioned this pull request Sep 2, 2020
4 tasks
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #34891
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
PR-URL: #34891
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
@codebytere codebytere mentioned this pull request Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants