Skip to content

Conversation

@addaleax
Copy link
Member

We are using ObjectPrototypeToString() as a cross-context brand check
for built-in errors, but weren’t making sure to set that when
deserializing errors back into JS objects.

Fix that by setting [Symbol.toStringTag] manually, to make sure that
multiple serialize-and-deserialize cycles keep giving the same result.

Fixes: #34309

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

We are using `ObjectPrototypeToString()` as a cross-context brand check for built-in errors, but weren’t making sure to set that when deserializing errors back into JS objects. Fix that by setting `[Symbol.toStringTag]` manually, to make sure that multiple serialize-and-deserialize cycles keep giving the same result. Fixes: nodejs#34309
@addaleaxaddaleax added worker Issues and PRs related to Worker support. lts-watch-v12.x labels Jul 11, 2020
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 11, 2020

CI: https://ci.nodejs.org/job/node-test-pull-request/32280/ (:heavy_check_mark:)

@ZYSzysZYSzys added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Jul 12, 2020
addaleax added a commit that referenced this pull request Jul 14, 2020
We are using `ObjectPrototypeToString()` as a cross-context brand check for built-in errors, but weren’t making sure to set that when deserializing errors back into JS objects. Fix that by setting `[Symbol.toStringTag]` manually, to make sure that multiple serialize-and-deserialize cycles keep giving the same result. Fixes: #34309 PR-URL: #34310 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
@addaleax
Copy link
MemberAuthor

Landed in 1274f89

@addaleaxaddaleax deleted the worker-nested-error branch July 14, 2020 13:09
MylesBorins pushed a commit that referenced this pull request Jul 14, 2020
We are using `ObjectPrototypeToString()` as a cross-context brand check for built-in errors, but weren’t making sure to set that when deserializing errors back into JS objects. Fix that by setting `[Symbol.toStringTag]` manually, to make sure that multiple serialize-and-deserialize cycles keep giving the same result. Fixes: #34309 PR-URL: #34310 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Jul 14, 2020
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
We are using `ObjectPrototypeToString()` as a cross-context brand check for built-in errors, but weren’t making sure to set that when deserializing errors back into JS objects. Fix that by setting `[Symbol.toStringTag]` manually, to make sure that multiple serialize-and-deserialize cycles keep giving the same result. Fixes: #34309 PR-URL: #34310 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
cjihrig pushed a commit that referenced this pull request Jul 23, 2020
We are using `ObjectPrototypeToString()` as a cross-context brand check for built-in errors, but weren’t making sure to set that when deserializing errors back into JS objects. Fix that by setting `[Symbol.toStringTag]` manually, to make sure that multiple serialize-and-deserialize cycles keep giving the same result. Fixes: #34309 PR-URL: #34310 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
addaleax added a commit that referenced this pull request Sep 22, 2020
We are using `ObjectPrototypeToString()` as a cross-context brand check for built-in errors, but weren’t making sure to set that when deserializing errors back into JS objects. Fix that by setting `[Symbol.toStringTag]` manually, to make sure that multiple serialize-and-deserialize cycles keep giving the same result. Fixes: #34309 PR-URL: #34310 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
addaleax added a commit that referenced this pull request Sep 22, 2020
We are using `ObjectPrototypeToString()` as a cross-context brand check for built-in errors, but weren’t making sure to set that when deserializing errors back into JS objects. Fix that by setting `[Symbol.toStringTag]` manually, to make sure that multiple serialize-and-deserialize cycles keep giving the same result. Fixes: #34309 PR-URL: #34310 Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Yongsheng Zhang <[email protected]> Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]>
@codebyterecodebytere mentioned this pull request Sep 28, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.workerIssues and PRs related to Worker support.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Nested worker threads fail to produce meaningful error messages

7 participants

@addaleax@nodejs-github-bot@jasnell@lpinca@targos@ZYSzys@MylesBorins