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 interop stack trace unwind (#1202) #1213
Conversation
* remove quick patch from JintTryStatement introduced in sebastienros#853 * implement "ideal" solution described in sebastienros#853 (try/finally in Engine.Call and Engine.Construct) * change behaviour of JavaScriptException.SetJavaScriptCallstack() so that it will not overwrite the existing stack trace unless explicitly told to * add test
var stack = engine.Evaluate(Script).AsString(); | ||
EqualIgnoringNewLineDifferences(@" at Error <anonymous>:3:21 | ||
at throwIt (message) <anonymous>:3:15 | ||
at <anonymous>:11:5", stack); |
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.
Note: this stack trace still isn't quite right, as the Error ctor should not be included. However, that is a separate issue (there are other tests that expect it to be there), which I may try to address in a subsequent PR.
@@ -1030,7 +1030,8 @@ private static ObjectInstance ToObjectNonObject(Realm realm, JsValue value) | |||
{ | |||
referencedName ??= "unknown"; | |||
var message = $"Cannot read property '{referencedName}' of {o}"; | |||
throw new JavaScriptException(engine.Realm.Intrinsics.TypeError, message).SetJavaScriptCallstack(engine, sourceNode.Location); | |||
throw new JavaScriptException(engine.Realm.Intrinsics.TypeError, message) | |||
.SetJavaScriptCallstack(engine, sourceNode.Location, overwriteExisting: true); |
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.
In this case we need to overwrite the existing stack trace (the one that is created within the Error ctor) in order to use the correct location (i.e. the passed source.Location
).
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.
Great work here, thanks for the contribution!
fixes #1202