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

console should attempt to report if monitored app exits/panics, if possible #179

Open
pnkfelix opened this issue Nov 4, 2021 · 3 comments
Labels
A-instrumentation Area: instrumentation. C-api Crate: console-api C-console Crate: console. C-subscriber Crate: console-subscriber. E-medium Effort: medium. S-feature Severity: feature. This is adding a new feature.

Comments

@pnkfelix
Copy link
Contributor

pnkfelix commented Nov 4, 2021

I was staring at a terminal wondering why console was failing to connect.

Then I eventually switched to the other screen, and realized that the app I was monitoring had panicked.

Ideally, we could use a RAII object to send a message to the console when an app quits cleanly or panics cleanly, right?

(Having said that, I don't know off hand if it is generally a bad idea to incur new network traffic in destructors, since that does seem like it could invite a panic from a destructor situation in some circumstances. So if this is just a bad idea, feel free to close this feature request.)

@pnkfelix pnkfelix added the S-feature Severity: feature. This is adding a new feature. label Nov 4, 2021
@hawkw
Copy link
Member

hawkw commented Nov 4, 2021

The biggest issue, I think, is that if the program is exiting, and we drop some guard that's held in main, we have almost certainly already dropped the Tonic gRPC server, and closed the connection to the client. So, informing the client that the process is exiting wouldn't be possible in that case, since connections from the console are client initiated.

We could potentially install a panic handler that records panics, though. This would also let us record when tasks panic even if those panics don't take down the whole application, such as when Tokio catches a panic. The biggest problem with that is that it could conflict with user-provided panic handlers, so we'd need to ensure that it was optional (and perhaps opt-in?) in case user code needs to set a panic handler.

@briankung
Copy link

At the least it might be nice to have an option to leave the terminated tasks in the console with a short reason for termination (if possible)

@hawkw
Copy link
Member

hawkw commented Dec 17, 2021

@briankung yeah, we can definitely do that --- it should be fairly straightforward to detect when individual tasks panic, when using a runtime where task panicking is captured. When a task panics, we can definitely record that it panicked using a panic hook, and maybe even include the panic message and stuff. Of course, this won't play nice with other user-defined panic hooks, but we can make it optional in cases where the user code also sets a panic hook.

The harder part is specifically handling the case where the entire program exits/aborts; as I mentioned, the gRPC server may have already been shut down by the time that we know the entire program is shutting down, so in that case, we can't easily do anything. We could try to work around this with a drop guard or something that's held in the main function that keeps the server alive until we unwind out of main, but it's still sort of a best-effort thing --- for example, if the program is compiled with panic="abort", there's nothing the console can do.

@hawkw hawkw added A-instrumentation Area: instrumentation. C-api Crate: console-api C-console Crate: console. C-subscriber Crate: console-subscriber. E-medium Effort: medium. labels Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-instrumentation Area: instrumentation. C-api Crate: console-api C-console Crate: console. C-subscriber Crate: console-subscriber. E-medium Effort: medium. S-feature Severity: feature. This is adding a new feature.
Projects
None yet
Development

No branches or pull requests

3 participants