-
Notifications
You must be signed in to change notification settings - Fork 410
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
feat: Print unexpected panics to standard error #601
Conversation
This will print to standard error for unexpected panics and then let `human_panic` handle panics, just like before.
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.
thanks so much for this @drager! is there any way to test this? (i'm not sure..) could you show an example of what this looks like when a panic happens?
// Then call human_panic. | ||
let file_path = human_panic::handle_dump(&meta, info); | ||
human_panic::print_msg(file_path, &meta) | ||
.expect("human-panic: printing error message to console failed"); |
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.
if you'd like to use ? here instead of expect we can add returning a Result from main to this PR in another commit! if not, that's OK :)
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.
I'm not sure we can do that since set_hook wont accept a Result
as a return type for the hook. It only accepts ()
.
@ashleygwilliams Yeah, you could do this by just panicing somewhere in the wasm-pack codebase. Here's what it looks like: |
per convo in discord, @drager is gonna do the refactor of main to return a result by monday so we can land this in the 0.8.0 release i intend to do on that day. if not, we can land this as is for the release and refactor later :) |
@ashleygwilliams I'm not sure we can refactor that |
@drager let's just land this as is then! |
This will print to standard error for unexpected panics and then let
human_panic
handle panics, just like before.This will fix #562.