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 #114

Closed
llogiq opened this issue Aug 30, 2021 · 7 comments · Fixed by #168
Closed

Custom panic handler #114

llogiq opened this issue Aug 30, 2021 · 7 comments · Fixed by #168
Assignees

Comments

@llogiq
Copy link
Contributor

llogiq commented Aug 30, 2021

Required Functionality
Whenever synth panics, the user should get the option to send an automated bug message with another option to give their contact info so we can get back to them. This will help our users be more active when bugs occur and help us get more and better bug reports.

Proposed Solution
A custom panic handler can get some information (synth version, operating system, etc.) to format and send to an endpoint we need to set up.

Use case
Improve our reaction to bugs, make synth better faster

@juniorbassani
Copy link
Contributor

Hey, can I work on this?

@christos-h
Copy link
Member

Sure @juniorbassani ! Do you have ideas for the design?

@juniorbassani
Copy link
Contributor

Sure @juniorbassani ! Do you have ideas for the design?

I was thinking of setting up a panic hook asking if the user agrees on sending the panic info and if they want to send some contact info. Maybe sending the panic cause could be enabled by default on the installation script. As for the contact information, we would need to be a bit careful as it is personal data.

After that, we can send a post request to some endpoint (I see we're already using the posthog service).

@llogiq
Copy link
Contributor Author

llogiq commented Sep 11, 2021

I think collecting the stack trace of the panic plus some system information and perhaps a sanitized form of the arguments (just so we know if the user e.g. exported to a database) would be quite helpful.

Of course the user should have final say in what data gets sent.

@christos-h
Copy link
Member

christos-h commented Sep 13, 2021

Maybe sending the panic cause could be enabled by default on the installation script.

@juniorbassani Yeah I think if telemetry is enabled, this should get sent off by default. We just need to do a sanity check and make sure that no sensitive information is being leaked (things like field names, collection names, db uris etc.)

@juniorbassani
Copy link
Contributor

I'm a bit uncertain on which option to choose for collecting the stack trace:

  1. Use the standard backtrace api (currently unstable).
  2. Use the backtrace crate.

Since we're on nightly, we could just use the standard backtrace api. The downside is that it's probably not going to be stabilized soon.
If we want something more stable, we could go with option 2, although that would require an additional dependency.

@llogiq
Copy link
Contributor Author

llogiq commented Sep 14, 2021

I don't think either would be a problem. Personally, I'd likely choose the backtrace crate, because I've seen unstable features break our build in the past.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants