Skip to content

fix(screenshots): Attach screenshots to the error envelope - #2835

Open
bitsandfoxes wants to merge 18 commits into
mainfrom
fix/screenshots
Open

bitsandfoxes wants to merge 18 commits into
mainfrom
fix/screenshots

Conversation

@bitsandfoxes

@bitsandfoxes bitsandfoxes commented Sep 4, 2026 •

Copy link
Copy Markdown
Contributor

Screenshots are now attached to the event they belong to and sent in its envelope, instead of being sent as a separate envelope afterwards.

Sending them separately meant a screenshot got orphaned whenever its event was dropped, e.g. by sampling or rate limiting. Capturing also blocked the main thread: reading the frame back from the GPU and encoding it to JPG stalled the frame for every screenshot (up to ~130 ms per screenshot at High on a Pixel 6, ~50 ms on an iPhone 18 Pro). And only some events got a screenshot at all.

How it works now:

  • ScreenshotEventProcessor adds a SentryScreenshotAttachmentContent to the event's hint. Events captured while a screenshot is still being taken share it.
  • The screenshot gets taken at the end of the frame the event was captured in, and skipped if none of the events sharing it made it past BeforeSend and sampling. Where supported, it gets read back via AsyncGPUReadback and encoded to JPG on a background thread right away, so the main thread only issues GPU work and no raw frames pile up in memory. With SetBeforeSendScreenshot set, or without async readback (e.g. on the WebBackgroundWorker), it's read and encoded on the main thread as before.
  • The attachment content waits for the screenshot when the SDK serializes the envelope on its background worker. Its stream reports no length, so Envelope.FromEvent doesn't block on the capturing thread. It never waits on the main thread and gives up 1 s after the event was captured if the main thread stops rendering; a capture stuck that long gets replaced.
  • Batch mode no longer attaches screenshots, since the end of the frame never comes. Feedback only gets one when requested via addScreenshot.

Behavior changes:

  • SetBeforeCaptureScreenshot now runs when the event gets captured, which can be a background thread. Events from background threads get a screenshot too.
  • SetBeforeSendScreenshot runs once per screenshot, receiving the first of the events sharing it that didn't get dropped.

Known gap: when a screenshot is discarded, times out or fails, the event carries an empty screenshot.jpg. sentry-dotnet only skips empty attachments whose stream reports a length of 0, which this one can't do without blocking. This gets fixed upstream in sentry-dotnet.

Measured on an earlier revision of this PR, on a Pixel 6 and an iPhone 18 Pro (development builds, 1 error/s):

  • Frames over 20 ms: 19–22 down to ~1 per 20 screenshots on the Pixel, 14–19 down to 0 on the iPhone.
  • Every sent event carries its screenshot, against about a quarter before.

Docs: getsentry/sentry-docs#19972

🤖 Generated with Claude Code

bitsandfoxes and others added 5 commits September 4, 2026 14:14
Attaching a screenshot blocked the thread the error happened on for the
GPU readback and the JPG encode, ~35 ms per error on an iPhone at High
quality. Read the cached frame back via AsyncGPUReadback and attach it
as content that only encodes once the SDK serializes the envelope on its
background worker. The screenshot still rides in the event's envelope.

Errors on background threads now get a screenshot too where async
readback is supported. Also stop the refresh coroutine once the cache is
disposed, so Close no longer leaks a render texture or throws when
called off the main thread.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Builds a harness from the sample that runs screenshot configurations
round-robin against a baseline with screenshots disabled, records frame
timings, capture costs and encodes on the SDK's worker, and checks that
sent events carry their screenshot. A report script summarizes a run
against its baseline.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@bitsandfoxes bitsandfoxes changed the title poc: capture screenshots with error envelope fix(screenshots): Attach screenshots to the error envelope Oct 14, 2026
@bitsandfoxes
bitsandfoxes marked this pull request as ready for review October 14, 2026 15:54
@bitsandfoxes
bitsandfoxes marked this pull request as draft October 14, 2026 15:59

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

Comment thread src/Sentry.Unity/SentryScreenshotCache.cs Outdated
Comment thread src/Sentry.Unity/SentryScreenshotCache.cs Outdated
Comment thread src/Sentry.Unity/SentryMonoBehaviour.Screenshot.cs Outdated
bitsandfoxes and others added 2 commits October 15, 2026 11:08
Keep the change focused on how screenshots get attached to events. The
device benchmark, the profiler markers and the encode timing log only
served measuring it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Replace the periodically refreshed frame cache with screenshots requested
by the event and taken at the end of the frame it was captured in. Idle
frames no longer pay for screenshots, and the screenshot shows the frame
the error happened in. Events in quick succession share a screenshot.

Read back into pooled native buffers and encode them on the SDK's worker,
so the main thread only issues the GPU work. The capture now happens at
screen resolution and gets scaled down in a blit, as capturing straight
into a smaller target cropped the screenshot.

Add SetBeforeSendScreenshotPixels, which edits the raw pixels on the
worker right before encoding. Deprecate SetBeforeSendScreenshot, whose
Texture2D ties encoding to the main thread, and apply it once per
screenshot. Default ScreenshotQuality to Low.

Refs #2835
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bitsandfoxes added a commit to getsentry/sentry-docs that referenced this pull request Oct 16, 2026
The Unity SDK now takes the screenshot at the end of the frame the error
happened in instead of periodically. Drop ScreenshotCaptureInterval,
document SetBeforeSendScreenshotPixels and the Low default quality, and
mark SetBeforeSendScreenshot as deprecated.

Refs getsentry/sentry-unity#2835
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit caac23e. Configure here.

finally
{
RenderTexture.ReleaseTemporary(renderTexture);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Readback releases texture too early

Medium Severity

CaptureAsync calls RenderTexture.ReleaseTemporary as soon as AsyncGPUReadback.Request returns. Unity only queues that readback; the source must stay alive until the callback runs. A pooled screen-sized texture can be reused first, so the readback fails or returns overwritten pixels and the event gets a missing or corrupted screenshot.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit caac23e. Configure here.

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.

1 participant