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

Custom panic handler wipes out backtrace rendering #4884

Open
stuhood opened this issue Sep 21, 2017 · 2 comments

Comments

@stuhood
Copy link
Member

commented Sep 21, 2017

Inserting a panic in the native engine on master, you will see:

$ ./pants list 3rdparty/:
CRITICAL] panic at 'not yet implemented', src/rust/engine/src/scheduler.rs:40
CRITICAL] Please file a bug at https://github.com/pantsbuild/pants/issues.
fatal runtime error: failed to initiate panic, error 5
Fatal Python error: Aborted

...due to the custom panic handler installed by

def set_panic_handler(self):


Passing RUST_BACKTRACE=1 now disables the custom panic handler, but unfortunately this means that we won't get anything about the panic recorded to pantsd.log.

I think that we might need a middle ground whereby we:

  1. add support to the custom handler for actually successfully rendering the panic to the log
  2. either enable RUST_BACKTRACE by default, or override/ignore the flag in the custom handler
@stuhood

This comment has been minimized.

Copy link
Member Author

commented Mar 14, 2018

Going to reopen this, because we're still hiding the actual panic message. AFAICT, the panic handler we have installed currently does not work.

@stuhood

This comment has been minimized.

Copy link
Member Author

commented Apr 20, 2018

Updated the description to describe an adjusted goal.

@stuhood stuhood removed their assignment Apr 20, 2018

gshuflin added a commit to gshuflin/pants that referenced this issue Aug 28, 2019
Fix custom panic handler (pantsbuild#4884)
The custom panic handler was previously assuming that the payload of a
panic was of type &str, and calling .unwrap() on the result of
downcast_ref::<&str>. However, depending on how panic! was invoked, the
type of the payload may be &str, String, or in principle some other
type. Since there are cases in the pants codebase where panic! is
invoked with format!-style macros, resuling in a String rather than &str
payload, these panics induced an additional panic in the .unwrap(),
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message. This is fixed by removing
the .unwrap() and explicitly handling all possible cases.
gshuflin added a commit to gshuflin/pants that referenced this issue Aug 29, 2019
Fix custom panic handler (pantsbuild#4884)
The custom panic handler was previously assuming that the payload of a
panic was of type &str, and calling .unwrap() on the result of
downcast_ref::<&str>. However, depending on how panic! was invoked, the
type of the payload may be &str, String, or in principle some other
type. Since there are cases in the pants codebase where panic! is
invoked with format!-style macros, resuling in a String rather than &str
payload, these panics induced an additional panic in the .unwrap(),
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message. This is fixed by removing
the .unwrap() and explicitly handling all possible cases.
gshuflin added a commit to gshuflin/pants that referenced this issue Aug 29, 2019
Fix custom panic handler (pantsbuild#4884)
The custom panic handler was previously assuming that the payload of a
panic was of type &str, and calling .unwrap() on the result of
downcast_ref::<&str>. However, depending on how panic! was invoked, the
type of the payload may be &str, String, or in principle some other
type. Since there are cases in the pants codebase where panic! is
invoked with format!-style macros, resuling in a String rather than &str
payload, these panics induced an additional panic in the .unwrap(),
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message. This is fixed by removing
the .unwrap() and explicitly handling all possible cases.
gshuflin added a commit to gshuflin/pants that referenced this issue Aug 29, 2019
Fix custom panic handler (pantsbuild#4884)
The custom panic handler was previously assuming that the payload of a
panic was of type &str, and calling .unwrap() on the result of
downcast_ref::<&str>. However, depending on how panic! was invoked, the
type of the payload may be &str, String, or in principle some other
type. Since there are cases in the pants codebase where panic! is
invoked with format!-style macros, resuling in a String rather than &str
payload, these panics induced an additional panic in the .unwrap(),
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message. This is fixed by removing
the .unwrap() and explicitly handling all possible cases.
gshuflin added a commit to gshuflin/pants that referenced this issue Aug 30, 2019
Fix custom panic handler (pantsbuild#4884)
The custom panic handler was previously assuming that the payload of a
panic was of type &str, and calling .unwrap() on the result of
downcast_ref::<&str>. However, depending on how panic! was invoked, the
type of the payload may be &str, String, or in principle some other
type. Since there are cases in the pants codebase where panic! is
invoked with format!-style macros, resuling in a String rather than &str
payload, these panics induced an additional panic in the .unwrap(),
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message. This is fixed by removing
the .unwrap() and explicitly handling all possible cases.
gshuflin added a commit to gshuflin/pants that referenced this issue Aug 30, 2019
Fix custom panic handler (pantsbuild#4884)
The custom panic handler was previously assuming that the payload of a
panic was of type &str, and calling .unwrap() on the result of
downcast_ref::<&str>. However, depending on how panic! was invoked, the
type of the payload may be &str, String, or in principle some other
type. Since there are cases in the pants codebase where panic! is
invoked with format!-style macros, resuling in a String rather than &str
payload, these panics induced an additional panic in the .unwrap(),
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message. This is fixed by removing
the .unwrap() and explicitly handling all possible cases.
gshuflin added a commit to gshuflin/pants that referenced this issue Sep 3, 2019
Fix custom panic handler (pantsbuild#4884)
The custom panic handler was previously assuming that the payload of a
panic was of type &str, and calling .unwrap() on the result of
downcast_ref::<&str>. However, depending on how panic! was invoked, the
type of the payload may be &str, String, or in principle some other
type. Since there are cases in the pants codebase where panic! is
invoked with format!-style macros, resuling in a String rather than &str
payload, these panics induced an additional panic in the .unwrap(),
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message. This is fixed by removing
the .unwrap() and explicitly handling all possible cases.
stuhood added a commit that referenced this issue Sep 4, 2019
Fix custom panic handler (#4884) (#8219)
### Problem:
cf. #4884, the custom panic handler was previously assuming that the payload of a panic was of type `&str`, and calling `.unwrap()` on the result of
`downcast_ref::<&str>`. However, depending on how `panic!` was invoked, the
type of the payload may be `&str`, `String`, or in principle some other
type. Since there are cases in the pants codebase where `panic!` is
invoked with format!-style macros, resulting in a String rather than `&str`
payload, these panics induced an additional panic in the `.unwrap()`,
which displayed "thread panicked while processing panic" and aborted,
rather than showing the proper error message.

### Solution:
This is fixed by removing the `.unwrap()` and explicitly handling all possible cases.

### Result:
`&str` and `String` panic messages are treated identically and the panic-in-panic-handler is not triggered.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
1 participant
You can’t perform that action at this time.