Skip to content
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

Make common.Throw add stacktraces the same as returning an error #2739

Merged
merged 2 commits into from
Oct 19, 2022

Conversation

mstoykov
Copy link
Contributor

No description provided.

@mstoykov mstoykov added the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Oct 18, 2022
@github-actions github-actions bot requested review from codebien and oleiade October 18, 2022 16:27
@mstoykov
Copy link
Contributor Author

mstoykov commented Oct 18, 2022

This in practice reverts #1775 as it turns out that with this change I can't get the stack trace to print it in the promise rejections.

I do find though that stack traces are way more useful than missing GoError and also GoError is there if you return an error from a function called by goja. So it makes them act differently - arguably not what we want about common.Throw.

js/common/util_test.go Outdated Show resolved Hide resolved
@mstoykov mstoykov requested a review from codebien October 19, 2022 09:12
Base automatically changed from promiseRejectionFixes to master October 19, 2022 10:53
@mstoykov mstoykov force-pushed the makeCommonThrowGreatAgain branch from 43dd4e2 to 9de8de0 Compare October 19, 2022 10:57
@mstoykov mstoykov requested a review from na-- October 19, 2022 11:03
@mstoykov mstoykov merged commit b60fe88 into master Oct 19, 2022
@mstoykov mstoykov deleted the makeCommonThrowGreatAgain branch October 19, 2022 12:06
@mstoykov mstoykov added this to the v0.41.0 milestone Oct 19, 2022
@mstoykov mstoykov removed the evaluation needed proposal needs to be validated or tested before fully implementing it in k6 label Oct 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants