err: document ABORT_ERR code by benjamingr · Pull Request #36319 · nodejs/node

benjamingr · 2020-11-29T18:13:42Z

We added an AbortError with the same code and name as the web's as
part of the consensus in #36084 but
never actually documented the error in our error codes list.

This PR adds the error code.

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
tests and/or benchmarks are included
documentation is changed or added
commit message follows commit guidelines

mcollina

lgtm

nodejs-github-bot · 2020-12-01T19:09:12Z

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

nodejs-github-bot · 2020-12-01T20:26:10Z

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

addaleax · 2020-12-01T20:29:15Z

Suggested change
### `ABRT_ERR`
### `ABRT_ERR`

?

Technically it's not 15.0 and this code existed on DOMException beforehand and users could craft it. The actual non DOMException AbortError is 15.2 - I am not sure what this should say?

I think it should say whatever is the first version in which Node.js actually throws errors with this code.

What version first had DOMException? 8?

I'm a bit confused about this code. Could you point me to where it is defined? I cannot find any occurence of ABRT_ERR in the source code.

@targos wow, we all missed the typo - it's ABORT_ERR and not ABRT_ERR.
I'll push a fix, the error is defined in two places - in internal/errors.j (as class AbortError extends Error {) and in lib/internal/per_context/domexception.js.

Oh, I didn't even think it could be a typo!
So I checked and v15.0.0 is indeed the first version where we throw errors with that code. I don't think we should mention versions where users could somehow do it themselves.

nodejs-github-bot · 2020-12-02T13:38:30Z

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

nodejs-github-bot · 2020-12-02T15:36:08Z

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

nodejs-github-bot · 2020-12-03T17:10:25Z

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

nodejs-github-bot · 2020-12-04T08:33:21Z

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

nodejs-github-bot · 2020-12-04T15:14:10Z

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

nodejs-github-bot · 2020-12-05T16:31:39Z

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

We added an `AbortError` with the same code and name as the web's as part of the consensus in nodejs#36084 but never actually documented the error in our error codes list. This PR adds the error code.

benjamingr · 2020-12-07T10:58:40Z

I think this probably needed a rebase 🤔

nodejs-github-bot · 2020-12-07T11:01:07Z

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

nodejs-github-bot · 2020-12-07T22:26:29Z

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

nodejs-github-bot · 2020-12-08T18:49:16Z

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

nodejs-github-bot · 2020-12-08T22:12:01Z

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

benjamingr · 2020-12-09T06:41:10Z

Landed in a2a2ff3

Trott · 2020-12-09T13:10:49Z

Landed in a2a2ff3

This commit was added to master without metadata. Not sure there's much to do at this point other than notify @nodejs/releasers so they can work around it in releases. (Actually, I'd be OK with a force push to fix if there was consensus on @nodejs/tsc. It's been 6 hours so far....)

gireeshpunathil · 2020-12-09T13:16:04Z

+1 to force-push.
(curious to know how did you catch this?)

Trott · 2020-12-09T13:26:12Z

(curious to know how did you catch this?)

When I rebase against master, I scan the new commits to see what's been landed. I never use the bring-master-up-to-date feature in node-core-utils and always do it manually before landing anything specifically so I can do this quick scan.

Trott

Belated LGTM

Trott · 2020-12-09T13:34:17Z

In addition to no metadata, it also has an invalid subsystem (err)`. Force pushing momentarily unless someone swoops in here or in IRC to say otherwise.

We added an `AbortError` with the same code and name as the web's as part of the consensus in #36084 but never actually documented the error in our error codes list. This PR adds the error code. PR-URL: #36319 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

Trott · 2020-12-09T13:39:05Z

Force pushed to fix metadata and subsystem stuff.

Landed in 13e2170

benjamingr · 2020-12-09T16:58:53Z

@Trott thanks, what did I do wrong?

benjamingruenbaum@Benjamins-MBP node % git node land 36319
✔ Done loading data for nodejs/node/pull/36319
----------------------------------- PR info ------------------------------------
Title err: document ABORT_ERR code (#36319)
⚠ Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch benjamingr:document-abort-error-code -> nodejs:master
Labels doc, errors
Commits 1
- err: document ABORT_ERR code
Committers 1
- Benjamin Gruenbaum <benjamingr@gmail.com>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36319
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
--------------------------------------------------------------------------------
⚠ Commits were pushed since the last review:
⚠ - err: document ABORT_ERR code
ℹ Last Full PR CI on 2020-12-08T22:12:01Z: https://ci.nodejs.org/job/node-test-pull-request/34862/
✔ Build data downloaded
ℹ This PR was created on Sun, 29 Nov 2020 18:13:42 GMT
✔ Approvals: 4
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/36319#pullrequestreview-540505050
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36319#pullrequestreview-540511955
✔ - Daijiro Wachi (@watilde): https://github.com/nodejs/node/pull/36319#pullrequestreview-540839835
✔ - Anna Henningsen (@addaleax): https://github.com/nodejs/node/pull/36319#pullrequestreview-542270150
--------------------------------------------------------------------------------
? This PR is not ready to land, do you want to continue? Yes
✔ No git am in progress
✔ No git rebase in progress
--------------------------------------------------------------------------------
? Do you want to try reset the local master branch to origin/master? Yes
⠙ Bringing origin/master up to date...From github.com:nodejs/node
* branch master -> FETCH_HEAD
✔ origin/master is now up-to-date
✔ Downloaded patch to /Users/benjamingruenbaum/Documents/Projects/node/.ncu/36319/patch
--------------------------------------------------------------------------------
Applying: err: document ABORT_ERR code
✔ Patches applied
--------------------------------------------------------------------------------
? There is only one commit in this PR.
do you want to amend the commit message? No

(Next commands are: I rant lint & tests and then pushed)

targos · 2020-12-09T17:13:39Z

do you want to amend the commit message? No

You have to answer "Yes" to this question, otherwise the commit message is not amended, so the metadata is not added to it

About the subsystem, I thought it was validated by git node land but maybe I don't remember correctly.

benjamingr · 2020-12-09T17:18:26Z

@targos

You have to answer "Yes" to this question, otherwise the commit message is not amended, so the metadata is not added to it

Thanks, my bad. I thought that it's not possible to land stuff with git node land without metadata.

I'll just check the commit log before pushing the next few times in case there is more stuff I miss.

We added an `AbortError` with the same code and name as the web's as part of the consensus in #36084 but never actually documented the error in our error codes list. This PR adds the error code. PR-URL: #36319 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net>

benjamingr added the doc Issues and PRs related to the documentations. label Nov 29, 2020

nodejs-github-bot added the errors Issues and PRs related to JavaScript errors originated in Node.js core. label Nov 29, 2020

benjamingr requested review from Trott, addaleax, jasnell and mcollina November 29, 2020 18:14

mcollina approved these changes Nov 29, 2020
View reviewed changes

lpinca approved these changes Nov 29, 2020
View reviewed changes

watilde approved these changes Nov 30, 2020
View reviewed changes

benjamingr added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 1, 2020

github-actionsBot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 1, 2020

This comment has been minimized.
Sign in to view

github-actionsBot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 1, 2020

benjamingr added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Dec 1, 2020

github-actionsBot 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 Dec 1, 2020

This comment has been minimized.
Sign in to view

benjamingr 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 Dec 1, 2020

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 1, 2020

addaleax approved these changes Dec 1, 2020
View reviewed changes

err: document ABORT_ERR code
dde4ae4
We added an `AbortError` with the same code and name as the web's as part of the consensus in nodejs#36084 but never actually documented the error in our error codes list. This PR adds the error code.

benjamingr force-pushed the document-abort-error-code branch from 6152951 to dde4ae4Compare December 7, 2020 10:58

benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2020

github-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 7, 2020

benjamingr closed this Dec 9, 2020

Trott reviewed Dec 9, 2020
View reviewed changes

This was referenced Dec 9, 2020
src: use using declarations consistently #36365
Closed
src: fix indentation in memory_tracker-inl.h #36425
Closed
src: remove identical V macro #36427
Closed
test: fix flaky test-repl #36415
Merged

targos mentioned this pull request Dec 22, 2020
v15.5.0 release proposal #36597
Merged

targos mentioned this pull request Jun 6, 2021
v14.17.1 release proposal #38948
Merged

-### `ABRT_ERR`
+### `ABRT_ERR`
+<!-- YAML
+added: v15.0.0
+-->

Uh oh!

Uh oh!

Conversation

benjamingr commented Nov 29, 2020

Checklist

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

This comment has been minimized.

nodejs-github-bot commented Dec 1, 2020

Uh oh!

nodejs-github-bot commented Dec 1, 2020

Uh oh!

addaleaxDec 1, 2020

Choose a reason for hiding this comment

Uh oh!

benjamingrDec 2, 2020

Choose a reason for hiding this comment

Uh oh!

targosDec 5, 2020

Choose a reason for hiding this comment

Uh oh!

benjamingrDec 5, 2020

Choose a reason for hiding this comment

Uh oh!

targosDec 6, 2020

Choose a reason for hiding this comment

Uh oh!

benjamingrDec 6, 2020

Choose a reason for hiding this comment

Uh oh!

targosDec 6, 2020

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Dec 2, 2020

Uh oh!

nodejs-github-bot commented Dec 2, 2020

Uh oh!

nodejs-github-bot commented Dec 3, 2020

Uh oh!

nodejs-github-bot commented Dec 4, 2020

Uh oh!

nodejs-github-bot commented Dec 4, 2020

Uh oh!

nodejs-github-bot commented Dec 5, 2020

Uh oh!

benjamingr commented Dec 7, 2020

Uh oh!

nodejs-github-bot commented Dec 7, 2020

Uh oh!

nodejs-github-bot commented Dec 7, 2020

Uh oh!

nodejs-github-bot commented Dec 8, 2020

Uh oh!

nodejs-github-bot commented Dec 8, 2020

Uh oh!

benjamingr commented Dec 9, 2020

Uh oh!

Trott commented Dec 9, 2020

Uh oh!

gireeshpunathil commented Dec 9, 2020

Uh oh!

Trott commented Dec 9, 2020

Uh oh!

Trott left a comment

Choose a reason for hiding this comment

Uh oh!

Trott commented Dec 9, 2020

Uh oh!

Trott commented Dec 9, 2020

Uh oh!

benjamingr commented Dec 9, 2020

Uh oh!

targos commented Dec 9, 2020

Uh oh!

benjamingr commented Dec 9, 2020

Uh oh!

Reviewers