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.

Sign up for GitHub

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

Jump to bottom

Print stacktrace on exception in init context or setup/teardown #1971

Merged
merged 6 commits into from
Apr 20, 2021
Merged

Print stacktrace on exception in init context or setup/teardown #1971

merged 6 commits into from
Apr 20, 2021

Conversation

mstoykov
Copy link
Contributor

also exit with error code 107 for both cases.

fixes #994

@mstoykov
Print stacktrace on exception in init context or setup/teardown
71e1a15 
also exit with error code 107 for both cases.

fixes #994
@mstoykov mstoykov requested review from imiric and na-- April 16, 2021 06:33
@mstoykov mstoykov mentioned this pull request Apr 16, 2021
Closed
na--
na-- reviewed Apr 16, 2021
View reviewed changes
Copy link
Member

@na-- na-- left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM, though I'd like to play around with it a bit before it's merged.

Should we try to release this in k6 v0.32.0?

cmd/run.go Show resolved Hide resolved
js/initcontext_test.go Show resolved Hide resolved
@mstoykov
Copy link
Contributor Author

I specifically didn't add it as v0.32.0, because I also wonder, it's pretty small and the worst case is ... somehow it breaks some stacktrace case?!?

On the other hand,today is the cut off date so ;)

imiric
imiric reviewed Apr 16, 2021
View reviewed changes
Copy link
Contributor

@imiric imiric left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, and works fine from what I could test.

Agree with Ned that a more complex test is needed. Maybe just replicating the example from #994 with setup() and teardown()?

👍 for releasing this in v0.32.0.

lib/types/types.go Outdated Show resolved Hide resolved
@na-- na-- added this to the v0.32.0 milestone Apr 16, 2021
@mstoykov mstoykov requested review from imiric and na-- April 16, 2021 12:47
@mstoykov mstoykov merged commit f2cbbac into master Apr 20, 2021
@mstoykov mstoykov deleted the fixGojaExceptionStacktraceEverywhere branch April 20, 2021 12:46
@na-- na-- mentioned this pull request Jun 7, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@na-- na-- na-- approved these changes

@imiric imiric imiric approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.32.0
Development

Successfully merging this pull request may close these issues.

Provide decent stack traces in setup code
3 participants
@mstoykov @imiric @na--