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: Also check crashpad pending directory on macOS when renderer process exits #592

Merged

Conversation

timfish
Copy link
Collaborator

@timfish timfish commented Nov 18, 2022

Closes #591

On macOS, if you call webContents.forcefullyCrashRenderer() the resulting minidump ends up in the pending subdirectory and doesn't get copied to completed until the app restarts. This causes it to be classed as a main process crash.

This PR means we now check the pending directory for minidumps on macOS and adds a test that ensures minidumps are correctly handled when caused by webContents.forcefullyCrashRenderer().

@timfish timfish force-pushed the fix/also-check-crashpad-pending-directory branch from 59d5ebb to 52c4cbd Compare November 18, 2022 13:00
@timfish timfish marked this pull request as ready for review November 18, 2022 14:35
@timfish timfish changed the title fix: Also check crashpad pending directory when renderer process exits fix: Also check crashpad pending directory on macOS when renderer process exits Nov 20, 2022
src/main/integrations/sentry-minidump/minidump-loader.ts Outdated Show resolved Hide resolved
const files = await readDirAsync(path);
found.push(...files.map((file) => join(path, file)));
} catch (_) {
//
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we debug log out something here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The catch is to cater for the directory not existing but it doesn't matter if it's missing

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh I see, thanks for clarifying, that kinda flew over my head :p

@timfish timfish merged commit 17b8e56 into getsentry:master Nov 21, 2022
@timfish timfish deleted the fix/also-check-crashpad-pending-directory branch November 21, 2022 13:03
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.

Minidump is not picked up after webContents.forcefullyCrashRenderer()
2 participants