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

desktop: Implement crash report dialog when a panic happens #9205

Merged
merged 8 commits into from
Jan 18, 2023

Conversation

Dinnerbone
Copy link
Contributor

When a panic happens:

image

Pressing Yes takes you to this new issue template, prefilled

desktop/src/main.rs Outdated Show resolved Hide resolved
@Bale001
Copy link
Member

Bale001 commented Jan 18, 2023

We should also include the AVM2 stack trace in the crash report, which will be helpful for AS3 bugs

@Dinnerbone
Copy link
Contributor Author

@Bale001 how safe is our callback collection? Is there any possibility of it panicking during access?

@Bale001
Copy link
Member

Bale001 commented Jan 18, 2023

@Bale001 how safe is our callback collection? Is there any possibility of it panicking during access?

It's safe to access - StaticCallstack::avm2 will simply do nothing if it can't access the callstack

@Dinnerbone
Copy link
Contributor Author

New example form

@firoball
Copy link

Small thing: Is it really wanted that "additional information" is already pre-filled with an example in the form? People might overlook (or not even care) and just submit whatever is already listed there (Radeon adapter etc.). When reviewing the issue later on, this could cause confusions.

@Dinnerbone
Copy link
Contributor Author

They can't just hit submit as some fields are required but empty. If we don't prefill system info, I'm not sure we'd ever get that info to honest

@Bale001
Copy link
Member

Bale001 commented Jan 18, 2023

@Dinnerbone My only concern is that StaticCallstack::avm2 currently still calls the closure even with an empty stack trace, so we should make sure the stack trace isn't empty before including it (Or modify StaticCallstack::avm2 to not call the closure with an empty stack trace)

@Dinnerbone
Copy link
Contributor Author

Small thing: Is it really wanted that "additional information" is already pre-filled with an example in the form? People might overlook (or not even care) and just submit whatever is already listed there (Radeon adapter etc.). When reviewing the issue later on, this could cause confusions.

I just re-read this and maybe there's some confusion - the info prefilled is the users info, not example info. The link I gave as an example for this PR is what I got when I hit the "yes" button, but other users will get their own info.

@firoball
Copy link

Small thing: Is it really wanted that "additional information" is already pre-filled with an example in the form? People might overlook (or not even care) and just submit whatever is already listed there (Radeon adapter etc.). When reviewing the issue later on, this could cause confusions.

I just re-read this and maybe there's some confusion - the info prefilled is the users info, not example info. The link I gave as an example for this PR is what I got when I hit the "yes" button, but other users will get their own info.

Ah, got it. So I am seeing your system details only since it is preview for this PR.
No issue then. Sorry for the confusion.

@Dinnerbone Dinnerbone requested a review from n0samu January 18, 2023 20:49
@Dinnerbone
Copy link
Contributor Author

Dinnerbone commented Jan 18, 2023

New example form

Turns into this: Dinnerbone#22

@Dinnerbone Dinnerbone enabled auto-merge (rebase) January 18, 2023 23:41
@Dinnerbone Dinnerbone merged commit f83c573 into ruffle-rs:master Jan 18, 2023
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.

4 participants