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

Supply PanicInfo to Catchers where applicable #2703

Closed
2 tasks done
phayes opened this issue Jan 17, 2024 · 2 comments
Closed
2 tasks done

Supply PanicInfo to Catchers where applicable #2703

phayes opened this issue Jan 17, 2024 · 2 comments
Labels
request Request for new functionality

Comments

@phayes
Copy link

phayes commented Jan 17, 2024

What's missing?

Most large complex applications have panics despite our best efforts to the contrary. Good visibility into these panics is the first step to fixing them. In terms of panics, good visibility means logging both the request that caused the panic along with the details of the panic. This is currently not possible in Rocket, as the Catcher functionality does not provide PanicInfo.

Ideal Solution

Ideally, A function decorated with #[catch] may take an additional argument: Option<PanicInfo> that will be either Some<PanicInfo> when the catcher is being caused because of a panic, or None when this is not the case.

The user will then be able to log both the panic, and details about the request that caused the panic, leading to greater visibility into the root cause of the panic and allowing the panic to be eliminated.

Why can't this be implemented outside of Rocket?

Good visibility into panics requires logging both request-information and panic information together. This isn't currently possible without support from Rocket.

Are there workarounds usable today?

The current workaround is to log the panic information (using std::panic::set_hook) and the request information (using #[catch]) separately. This isn't ideal because on a busy server these log messages might be quite far apart and hard to link which panic corresponds to which request.

Alternative Solutions

No response

Additional Context

There is a stack-overflow related to this here: https://stackoverflow.com/questions/76911134/is-it-possible-to-get-the-rust-rocket-500-error-message

System Checks

  • I do not believe that this feature can or should be implemented outside of Rocket.
  • I was unable to find a previous request for this feature.
@phayes phayes added the request Request for new functionality label Jan 17, 2024
@phayes
Copy link
Author

phayes commented Jan 17, 2024

Just realized that Rocket may not have a full PanicInfo but merely a Box<dyn Any + Send>> (from a std::panic::catch_unwind). Even a Box<dyn Any + Send>> that could be downcasted would be very useful.

@SergioBenitez
Copy link
Member

This is #749, which covers this case and many others.

@SergioBenitez SergioBenitez closed this as not planned Won't fix, can't repro, duplicate, stale Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
request Request for new functionality
Projects
None yet
Development

No branches or pull requests

2 participants