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

doc: add missing worker error by benjamingr · Pull Request #21947 · nodejs/node · GitHub

/ node Public

doc: add missing worker error - #21947

Closed
benjamingr wants to merge 3 commits into
nodejs:masterfrom
benjamingr:add-missing-error
ConversationCommits (3)ChecksFiles changed
Closed

doc: add missing worker error#21947
benjamingr wants to merge 3 commits into
nodejs:masterfrom
benjamingr:add-missing-error

Conversation

benjamingr commented Jul 23, 2018

Copy link
Copy Markdown
Member

@ChALkeR noticed that ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER was missing from the docs. This PR adds it there based on the wording @addaleax used in the her PR.

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

doc: add missing worker error
959d342 
ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER was missing from the
docs - add it there based on the wording Anna used in the her PR.

PR-URL:
Reviewed-By:

nodejs-github-bot commented Jul 23, 2018

Copy link
Copy Markdown
Collaborator

@benjamingr build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/340/pipeline

nodejs-github-bot added doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core. labels Jul 23, 2018
benjamingr mentioned this pull request Jul 23, 2018
Closed
4 tasks
addaleax reviewed Jul 23, 2018
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thanks for doing the PR here!

Comment thread doc/api/errors.md Outdated

A SharedArrayBuffer object was transfered but we do not see a lifetime partner
object and it was not us who externalized it - we are not sure how to serialize
it because it's unclear how the memory is actually owned.

addaleax Jul 23, 2018

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

This description makes sense as a code comment, but it might not be ideal for the docs. How about this?

A SharedArrayBuffer object was encountered during serialization whose memory is not managed by the JavaScript engine or by Node. Such an SharedArrayBuffer can not be serialized.

This can only happen when native addons create SharedArrayBuffers in “externalized” mode, or put existing SharedArrayBuffer into externalized mode.

benjamingr Jul 23, 2018

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Sure, sounds good.

ChALkeR commented Jul 23, 2018 •
edited
Loading

Copy link
Copy Markdown
Member

@benjamingr Thanks!

In fact, this was noticed by a testcase from #21470 😉.

addaleax approved these changes Jul 23, 2018
View reviewed changes

addaleax left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Thank you!

vsemozhetbyt reviewed Jul 23, 2018
View reviewed changes
Comment thread doc/api/errors.md Outdated
<a id="ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER"></a>
### ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER

A `SharedArrayBuffer` whose memory is not managed by the JavaScript engine or by Node.js was

vsemozhetbyt Jul 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Line length here and below)

vsemozhetbyt reviewed Jul 23, 2018
View reviewed changes
Comment thread doc/api/errors.md Outdated
### ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER

A `SharedArrayBuffer` whose memory is not managed by the JavaScript engine or by Node.js was
encountered during serialization. Such a `SharedArrayBuffer` can not be serialized.

vsemozhetbyt Jul 23, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

can not -> cannot?

vsemozhetbyt commented Jul 23, 2018

Copy link
Copy Markdown
Contributor

Node.js Collaborators, please, add 👍 here if you approve fast-tracking.

benjamingr commented Jul 23, 2018

Copy link
Copy Markdown
Member Author

New Lite CI after fixes: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/341/pipeline

addaleax added the fast-track PRs that do not need to wait for 48 hours to land. label Jul 23, 2018

addaleax commented Jul 27, 2018

Copy link
Copy Markdown
Member

Landed in f42aa32

addaleax closed this Jul 27, 2018
addaleax pushed a commit that referenced this pull request Jul 27, 2018
doc: add missing worker error
f42aa32 
ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER was missing from the
docs - add it there based on the wording Anna used in the her PR.

PR-URL: #21947
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this pull request Jul 31, 2018
doc: add missing worker error
1f9570b 
ERR_TRANSFERRING_EXTERNALIZED_SHAREDARRAYBUFFER was missing from the
docs - add it there based on the wording Anna used in the her PR.

PR-URL: #21947
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos mentioned this pull request Jul 31, 2018
Merged
This was referenced Aug 1, 2018
Closed
Closed
Closed
Closed
Closed
beevelop mentioned this pull request Aug 2, 2018
Closed
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
lpinca lpinca approved these changes
cjihrig cjihrig approved these changes
JungMinu JungMinu approved these changes
+1 more reviewer
vsemozhetbyt vsemozhetbyt approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. errors Issues and PRs related to JavaScript errors originated in Node.js core. fast-track PRs that do not need to wait for 48 hours to land.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

Back | FazBrowse Home | New Git URL