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

UWP Crash reporter #27438

Merged
merged 5 commits into from Aug 4, 2020
Merged

UWP Crash reporter #27438

merged 5 commits into from Aug 4, 2020

Conversation

paulrouget
Copy link
Contributor

@paulrouget paulrouget commented Jul 29, 2020

This is supposed to address #27167 and #26523. Also fix #27435.

These changes are still WIP as I found a few bugs, it needs more testing and the actual code to upload is not implemented yet. But I'd like to get an early feedback.

First, panics are caught via panic::set_hook instead of catch_unwind allowing us to catch more panics.
We also now report panics reported via the Embedder:Panic message.
Once the panic is caught, if possible, we try to recover.
I haven't found a way to recover when the panic is caught is a non-GL thread. We need a generic way to throw from the UWP code, and even trying to add a UnhandledEvent handler doesn't appear to work.

Once a panic is caught (even if we can not recover) a crash-report file is created, including the backtrace, stdout, and the current url.

If the app did not crash at that point, or after a restart if it did, we check if the crash report file is present, and if so, we present a panel to the user to allow them to upload the report. At that point the user can also add details to the report.

Screen Shot 2020-07-29 at 12 35 44

@highfive
Copy link

highfive commented Jul 29, 2020

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@jdm
Copy link
Member

jdm commented Jul 29, 2020

This looks really great!

paulrouget added 2 commits Aug 3, 2020
Stop using catch_unwind and use set_hook, allowing us to catch
more panics. Also reporting Embedder::Panic messages.
@paulrouget paulrouget changed the title WIP: UWP Crash reporter UWP Crash reporter Aug 3, 2020
@paulrouget
Copy link
Contributor Author

paulrouget commented Aug 3, 2020

@jdm can we land that preffed-off for now?

Next step will be to upload the crash report.

jdm
jdm approved these changes Aug 4, 2020
@jdm
Copy link
Member

jdm commented Aug 4, 2020

@bors-servo r+

@bors-servo
Copy link
Contributor

bors-servo commented Aug 4, 2020

📌 Commit 8759eb1 has been approved by jdm

@bors-servo
Copy link
Contributor

bors-servo commented Aug 4, 2020

Testing commit 8759eb1 with merge 52c9095...

@bors-servo
Copy link
Contributor

bors-servo commented Aug 4, 2020

☀️ Test successful - status-taskcluster
Approved by: jdm
Pushing 52c9095 to master...

@bors-servo bors-servo merged commit 52c9095 into servo:master Aug 4, 2020
2 checks passed
bors-servo added a commit that referenced this issue Aug 5, 2020
UWP: only show promoted preferences

This depends on changes made in #27438
Fix #27419
bors-servo added a commit that referenced this issue Aug 5, 2020
UWP: only show promoted preferences

This depends on changes made in #27438
Fix #27419
bors-servo added a commit that referenced this issue Aug 5, 2020
UWP: only show promoted preferences

This depends on changes made in #27438
Fix #27419
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 this pull request may close these issues.

5 participants