feat: Optionally attach screenshots

[timfish]

Closes #505

• Adds attachScreenshot property to main process options
• Adds Screenshots integration which is included by default so it can be enabled via the above option
• On JavaScript error, windows are captured as PNG and added as attachments
• Modified e2e server so that it captures different types of attachments rather than just minidumps
• Modified all e2e test events to use attachments rather than dumpFile

[AbhiPrasad]

Woah - amazing work Tim! Will review on Monday. Is there a example event on sentry.io with this - would like to see what it looks like.

[timfish]

Sorry my test app wasn't more interesting 🤣

[timfish]

Some other things that might be worth considering now or later:

• Should we only capture windows that are visible?
• Should the focussed window be the first screenshot?
• Should we try and track which renderer the error came from and make that the first screenshot?
• For main process errors, is capturing window screenshots that useful?
• Should we scale the screenshots so they're not full size?

[AbhiPrasad]

Should we only capture windows that are visible?

My gut instinct here is yes, as we want to capture what a user is seeing. Are there any possible scenarios where it might be useful to have it for non-visible windows?

Should the focused window be the first screenshot?

This feels like the correct behaviour to me.

Should we try and track which renderer the error came from and make that the first screenshot?

Why would we do this? Feels like un-needed complication.

For main process errors, is capturing window screenshots that useful?

I think we can stick with capturing screenshots for every single error, and just iterate on feedback.

Should we scale the screenshots so they're not full size?

As per the spec: Image size, if possible should stay below 2 MB but quality/size could be configurable. I think we should only adjust to make sure we stay within the size limit, otherwise we should not scale for now. We can revisit!

[timfish]

Should we try and track which renderer the error came from and make that the first screenshot?

Why would we do this? Feels like un-needed complication.

I would be looking at screenshots to see what the user was doing when the error occurred. I can imagine getting an event with multiple screenshots, one for each window and wanting to know which one is most relevant for the error I'm looking at.

Should the focused window be the first screenshot?

This feels like the correct behaviour to me.

Defaulting to the focussed window as the first screenshot could even suggest the error came from there.

In an ideal world we would tag the screenshots with some context like focus, and "errored", much like stack trace envelope API allows you to set which thread crashed. Could we put some of this into the file names?

screenshot-focus.png, screenshot-error.png, screenshot-focus-error.png, etc

I think we can stick with capturing screenshots for every single error, and just iterate on feedback.

Yep, it's probably good enough as it is for now!

[AbhiPrasad]

I would be looking at screenshots to see what the user was doing when the error occurred. I can imagine getting an event with multiple screenshots, one for each window and wanting to know which one is most relevant for the error I'm looking at.

Good point - it prevents you from looking back and forth.

In an ideal world we would tag the screenshots with some context like focus, and "errored", much like stack trace envelope API allows you to set which thread crashed. Could we put some of this into the file names?

At the current moment it wouldn't do anything (as we require the strict naming scheme to display in the product accordingly) but good suggestion.

I will raise these two points at our next internal SDK sync meeting, see what people think - and bring that feedback back here. In the mean time, I think we can ship with what we have!

[AbhiPrasad]

We just met and chatted - and the solution of using event.contexts was brought up.

So we would add an event context (maybe keyed under screenshot) that would have the information about what screenshots were focused or errored or anything else.

We might have to first make a PR against https://github.com/getsentry/develop/blob/master/src/docs/sdk/features.mdx#screenshots describing our ideal behaviour - and then inform the other teams working on this.