-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Fix error messages on errors created through the JSRT #4614
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
Fix error messages on errors created through the JSRT #4614
Conversation
lib/Jsrt/Jsrt.cpp
Outdated
| { | ||
| Js::JavascriptOperators::OP_SetProperty(newError, Js::PropertyIds::message, message, scriptContext); | ||
| Js::PropertyDescriptor desc; | ||
| desc.SetValue(message); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For ease-of-viewing, I'd appreciate if this also explicitly set configurable and writable to true, to match the spec.
boingoing
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
kfarnung
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should rebase against release/1.9
a0422ed to
f62d4d4
Compare
|
Updated the implementation a bit and rebased on 1.9 |
…h the JSRT Merge pull request #4614 from jackhorton:jsrt_error_message This will unblock the 01/14/18 node merge pump
…reated through the JSRT Merge pull request #4614 from jackhorton:jsrt_error_message This will unblock the 01/14/18 node merge pump
This will unblock the 01/14/18 node merge pump