You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
After #1697 we had a way to get the location of panic in js code in the form of a js stacktrace from goja ... or so we thought.
As explained in dop251/goja#179 (comment) this apparently hasn't really worked, to begin with, and I don't remember it actually being used in practice ... the whole panic wrapping is mostly a precaution on a super rare bug we have not been able to identify or fix. Even dop251/goja#214 wasn't actually caught with that even though the catching of the panics would've prevented it from killing k6.
For all we know that one issue could've been fixed in the meantime either by us or goja update 🤷♂️
The only way to fix it (as explained in the comment linked above) is for the golang code that panics to catch that and repanic with the goja stack at that time before it actually got unwinded. Doing this for each call ... will be very error-prone IMO and adding it to the bridge's Bind will not cover all cases.
Either way that adds at least 1 more defer per each call, which also will have some performance implications, although I doubt they will be ... big enough.
I am currently of the opinion that we should just drop the stack and if we start having problems where that would be useful again, to readd them then.
Alternatively maybe some discussion on whether goja can not unwind its stack for this case would be good ... although probably that will be quite a big change.
The text was updated successfully, but these errors were encountered:
This has been broken for years after some goja refactoring.
There doesn't seem to be a way for this to be fixed without a lot of
boilerplate all over the code. And this functionality has not been used
in the last few years in my experience.
closes#1906
After #1697 we had a way to get the location of panic in js code in the form of a js stacktrace from goja ... or so we thought.
As explained in dop251/goja#179 (comment) this apparently hasn't really worked, to begin with, and I don't remember it actually being used in practice ... the whole panic wrapping is mostly a precaution on a super rare bug we have not been able to identify or fix. Even dop251/goja#214 wasn't actually caught with that even though the catching of the panics would've prevented it from killing k6.
For all we know that one issue could've been fixed in the meantime either by us or goja update 🤷♂️
The only way to fix it (as explained in the comment linked above) is for the golang code that panics to catch that and repanic with the goja stack at that time before it actually got unwinded. Doing this for each call ... will be very error-prone IMO and adding it to the bridge's Bind will not cover all cases.
Either way that adds at least 1 more defer per each call, which also will have some performance implications, although I doubt they will be ... big enough.
I am currently of the opinion that we should just drop the stack and if we start having problems where that would be useful again, to readd them then.
Alternatively maybe some discussion on whether goja can not unwind its stack for this case would be good ... although probably that will be quite a big change.
The text was updated successfully, but these errors were encountered: