-
Notifications
You must be signed in to change notification settings - Fork 382
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Adding catch_panic anti-pattern #22
Conversation
@@ -48,7 +48,7 @@ language. | |||
|
|||
### Anti-patterns | |||
|
|||
* TODO thread + catch_panic for exceptions | |||
* [thread + catch_panic for exceptions](anti_patterns/catch_panic.md) |
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.
Could you change the title here to match the title in the description
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.
fixed in dc01184
@liamzdenek huge apologies for letting this and your other PR slip off my radar - it got pushed down my notifications and I forgot about it. I'll get some feedback for you before the end of the week. cc @cbreeden, @lfairy if you have time to comment here it would be appreciated. |
|
||
// the program continues running despite the panic | ||
println!("potential error: {:?}", result); | ||
assert!(result.is_err()); |
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.
The example could be improved by adding a function and which panics and catching the panic in the caller, then matching the Result. Describing the example you could show how by returning a Result, the Result-ness of the function is described in the signature.
|
||
Expected errors should not result in stack unwinding. Instead, expected errors | ||
should be handled through the Result and Option types. [The Rust Book's chapter | ||
on Error Handling](https://doc.rust-lang.org/book/error-handling.html) elaborates further on this. |
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.
Could you add why stack unwinding is bad? And perhaps the other disadvantages of using catch_unwind? Also, where is it appropriate to use catch_unwind
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.
Also: since Result::unwrap()
converts the error to a string, it's harder to distinguish between different kinds of errors than if we had matched the result directly.
Ping, @liamzdenek are you still alive? |
@liamzdenek any updates on this? |
Closing due to inactivity. Further additions to the PR can be made in #109. |
No description provided.