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

fix: Attempt to send minidumps for all process-gone reasons #586

Merged

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Nov 15, 2022

It was reported that webContents.forcefullyCrashRenderer() results in a minidump being generated but render-process-gone notifies that the process was killed.

We were ignoring all reasons which were not crashes and this resulted in ignoring some minidumps and then sending them on next app start and assuming they came from the main process.

This PR:

  • Looks for minidumps for all exit reasons
  • Includes the exit.reason in tags so customers can filter them

@timfish timfish mentioned this pull request Nov 15, 2022
@timfish timfish requested a review from AbhiPrasad November 15, 2022 11:29
@timfish timfish changed the title fix: Attempt to send minidumps for all process gone reasons fix: Attempt to send minidumps for all process-gone reasons Nov 15, 2022
@timfish timfish merged commit de9bf65 into getsentry:master Nov 15, 2022
@Cefold
Copy link

Cefold commented Nov 15, 2022

After webContents.forcefullyCrashRenderer() event, there was a minidump under pending folder, but it seems that Sentry cannot find it. It will upload it until next app restart, thus event.process will be classified as browser instead of renderer.

Sentry Logger [log]: 'renderer' process 'killed'
Sentry Logger [log]: Found 0 minidumps

@timfish
Copy link
Collaborator Author

timfish commented Nov 16, 2022

but it seems that Sentry cannot find it

It looks like the issue is that the minidump does not exist when render-process-gone is called. I need to do some more testing!

@timfish timfish deleted the fix/send-minidumps-for-all-reasons branch November 18, 2022 13:38
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.

3 participants