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

Add opt-in screenshot attachment for crash events captured on Windows/Linux #582

Merged
merged 11 commits into from
Jul 1, 2024

Conversation

tustanivsky
Copy link
Collaborator

@tustanivsky tustanivsky commented Jun 24, 2024

Related to #567

Copy link
Contributor

github-actions bot commented Jun 24, 2024

Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against cb5c7c0

@tustanivsky tustanivsky marked this pull request as ready for review June 24, 2024 05:53
@tustanivsky tustanivsky requested a review from bitsandfoxes June 24, 2024 05:54
Copy link
Contributor

@bitsandfoxes bitsandfoxes left a comment

Choose a reason for hiding this comment

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

This looks great!

CHANGELOG.md Outdated Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Changes like this reeeally make me think we should put a linter in the repo.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We'll probably cover this in scope of #575

tustanivsky and others added 3 commits June 27, 2024 16:59
Co-authored-by: Stefan Jandl <reg@bitfox.at>
# Conflicts:
#	CHANGELOG.md
@tustanivsky tustanivsky requested a review from bitsandfoxes June 27, 2024 14:13
Copy link
Contributor

@bitsandfoxes bitsandfoxes left a comment

Choose a reason for hiding this comment

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

Just to double check: In case the screenshot capture fails to write the file to disk, does the crashhandler deal with it gracefully?

@tustanivsky
Copy link
Collaborator Author

Just to double check: In case the screenshot capture fails to write the file to disk, does the crashhandler deal with it gracefully?

If a screenshot can't be captured for some reason crashpad will simply ignore that and capture crash as usual without adding a corresponding attachment. At the same time logs should contain info about screenshot capturing failure reason.

@tustanivsky tustanivsky merged commit 4dc30df into main Jul 1, 2024
16 checks passed
@tustanivsky tustanivsky deleted the feat/screenshots branch July 1, 2024 07:08
@bruno-garcia
Copy link
Member

Is this on the docs? Couldn't find any reference to screenshots there

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