Repository navigation
fix(screenshots): Attach screenshots to the error envelope - #2835
Open
bitsandfoxes wants to merge 18 commits into
Open
bitsandfoxes wants to merge 18 commits into
bitsandfoxes wants to merge 18 commits into
Conversation
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
marked this pull request as ready for review
October 14, 2026 15:54
bitsandfoxes
marked this pull request as draft
October 14, 2026 15:59
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>
…a missing frame can't stall every envelope
… runs once per shared screenshot
…ke them all at once
…on't skew the timeout
bitsandfoxes
marked this pull request as ready for review
October 23, 2026 22:13
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ 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); | ||
| } |
There was a problem hiding this comment.
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.
Reviewed by Cursor Bugbot for commit caac23e. Configure here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


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
Highon a Pixel 6, ~50 ms on an iPhone 18 Pro). And only some events got a screenshot at all.How it works now:
ScreenshotEventProcessoradds aSentryScreenshotAttachmentContentto the event's hint. Events captured while a screenshot is still being taken share it.BeforeSendand sampling. Where supported, it gets read back viaAsyncGPUReadbackand 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. WithSetBeforeSendScreenshotset, or without async readback (e.g. on theWebBackgroundWorker), it's read and encoded on the main thread as before.Envelope.FromEventdoesn'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.addScreenshot.Behavior changes:
SetBeforeCaptureScreenshotnow runs when the event gets captured, which can be a background thread. Events from background threads get a screenshot too.SetBeforeSendScreenshotruns 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):
Docs: getsentry/sentry-docs#19972
🤖 Generated with Claude Code