fix(screenshots): Attach screenshots to the error envelope

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+### Behavioral Changes
+
+- Screenshots are now sent in the same envelope as their event instead of separately, so they no longer get orphaned when the event gets dropped, e.g. by sampling or rate limiting. Events captured on background threads now get a screenshot as well, and events captured while a screenshot is still being taken share it. Screenshots are no longer attached in batch mode, nor to user feedback unless requested via `addScreenshot`. `SetBeforeCaptureScreenshot` can now get invoked on a background thread. `SetBeforeSendScreenshot` now runs once per screenshot, receiving the first of the events sharing it that didn't get dropped. ([#2835](https://github.com/getsentry/sentry-unity/pull/2835))
+
+### Fixes
+
+- Attaching a screenshot no longer stalls the main thread. Unless `SetBeforeSendScreenshot` is set, the frame gets read back from the GPU asynchronously and encoded on a background thread. ([#2835](https://github.com/getsentry/sentry-unity/pull/2835))
+
 ## 4.11.0
 
 ### Fixes

docs/agent-guides/integrations.md

@@ -22,6 +22,7 @@
 - Scene tracing replaces global `SceneManagerAPI.overrideAPI`; it overwrites application customization and logs a warning.
 - C# ANR monitoring pauses while backgrounded. Cocoa/native app-hang tracking can disable it to prevent duplicate reports.
 - Screenshot capture and view-hierarchy capture must run on Unity main thread.
+- Screenshots get attached to the event's hint as `SentryScreenshotAttachmentContent`, so they ride along the event's own envelope. Events captured while a screenshot is still being taken share it; it gets taken at end of frame. An async GPU readback gets encoded off the main thread right after it completes. The content waits for it when the envelope gets serialized on the SDK's worker; it never waits on the main thread and times out if the main thread stops rendering.
 
 ## Tests
 

src/Sentry.Unity/ScreenshotEventProcessor.cs

@@ -1,17 +1,23 @@
 using System;
 using System.Collections;
-using System.Threading;
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Threading.Tasks;
 using Sentry.Extensibility;
-using Sentry.Internal;
 using UnityEngine;
 
 namespace Sentry.Unity;
 
-public class ScreenshotEventProcessor : ISentryEventProcessor
+public class ScreenshotEventProcessor : ISentryEventProcessorWithHint
 {
     private readonly SentryUnityOptions _options;
     private readonly ISentryMonoBehaviour _sentryMonoBehaviour;
-    private volatile int _isCapturingScreenshot;
+
+    // Events share a screenshot until it's done, so frames can't pile up behind a slow encoder
+    private readonly object _lock = new();
+    private TaskCompletionSource<byte[]?>? _pending;
+    private readonly Stopwatch _pendingSince = new();
+    private List<SentryEvent> _pendingEvents = new();
 
     public ScreenshotEventProcessor(SentryUnityOptions sentryOptions) : this(sentryOptions, SentryMonoBehaviour.Instance) { }
 
@@ -21,43 +27,119 @@ internal ScreenshotEventProcessor(SentryUnityOptions sentryOptions, ISentryMonoB
         _sentryMonoBehaviour = sentryMonoBehaviour;
     }
 
-    public SentryEvent Process(SentryEvent @event)
+    public SentryEvent? Process(SentryEvent @event) => @event;
+
+    public SentryEvent? Process(SentryEvent @event, SentryHint hint)
     {
-        // Only ever capture one screenshot per frame
-        if (Interlocked.CompareExchange(ref _isCapturingScreenshot, 1, 0) == 0)
+        // Feedback attaches a screenshot only when asked to, see SentryUnitySdk.CaptureFeedback
+        if (@event.Contexts.Feedback is not null)
+        {
+            return @event;
+        }
+
+        try
+        {
+            if (_options.BeforeCaptureScreenshotInternal?.Invoke(@event) is false)
+            {
+                return @event;
+            }
+        }
+        catch (Exception e)
         {
-            _options.LogDebug("Starting coroutine to capture a screenshot.");
-            // Capture must run on the main thread after WaitForEndOfFrame (ReadPixels needs a complete frame), but the
-            // event processor pipeline is synchronous and may run on any thread - blocking here would deadlock when
-            // called from the main thread. So we capture in a coroutine and ship the screenshot as a separate envelope.
-            _sentryMonoBehaviour.QueueCoroutine(CaptureScreenshotCoroutine(@event));
+            _options.LogError(e, "The BeforeCaptureScreenshot callback threw an exception. Skipping the screenshot.");
+            return @event;
         }
 
+        Task<byte[]?> screenshot;
+        lock (_lock)
+        {
+            // A capture that never completes, e.g. while the main thread hangs, must not hold up later events
+            if (_pending is null || _pendingSince.Elapsed > SentryScreenshotAttachmentContent.CaptureTimeout)
+            {
+                // Its envelopes are past their deadline, so it doesn't need taking anymore
+                _pending?.TrySetResult(null);
+                _pending = new TaskCompletionSource<byte[]?>(TaskCreationOptions.RunContinuationsAsynchronously);
+                _pendingSince.Restart();
+                _pendingEvents = new List<SentryEvent>();
+                _sentryMonoBehaviour.QueueCoroutine(CaptureScreenshotCoroutine(_pending, _pendingEvents));
+            }
+
+            _pendingEvents.Add(@event);
+
+            screenshot = _pending.Task;
+        }
+
+        // The screenshot gets read when the SDK serializes the event's envelope on its worker
+        hint.Attachments.Add(new SentryAttachment(
+            AttachmentType.Default,
+            new SentryScreenshotAttachmentContent(screenshot, _options),
+            "screenshot.jpg",
+            "image/jpeg"));
+
         return @event;
     }
 
-    internal IEnumerator CaptureScreenshotCoroutine(SentryEvent @event)
+    private IEnumerator CaptureScreenshotCoroutine(TaskCompletionSource<byte[]?> screenshot, List<SentryEvent> events)
     {
-        _options.LogDebug("Screenshot capture triggered. Waiting for End of Frame.");
-
-        // WaitForEndOfFrame does not work in headless mode so we're making it configurable for CI.
-        // See https://docs.unity3d.com/6000.1/Documentation/ScriptReference/WaitForEndOfFrame.html
+        // Capturing mid-frame yields an incomplete image
         yield return WaitForEndOfFrame();
 
-        Texture2D? screenshot = null;
+        if (screenshot.Task.IsCompleted)
+        {
+            yield break;
+        }
+
+        // Event processors run before BeforeSend and sampling, which may have dropped the events
+        SentryEvent? @event;
+        lock (_lock)
+        {
+            @event = events.Find(e => e.IsCaptured);
+        }
+
+        if (@event is null)
+        {
+            _options.LogDebug("Skipping the screenshot. None of its events got captured.");
+            Complete(screenshot, null);
+            yield break;
+        }
+
         try
         {
-            if (!@event.IsCaptured)
+            // The Texture2D callback and the main thread worker need the screenshot right away
+            if (_options.BeforeSendScreenshotInternal is null && CanReadBackAsync())
             {
-                _options.LogDebug("Skipping screenshot for event {0}. Event was not captured.", @event.EventId);
-                yield break;
+                SentryScreenshot.CaptureAsync(_options, result => Complete(screenshot, result));
             }
+            else
+            {
+                Complete(screenshot, CaptureScreenshot(@event));
+            }
+        }
+        catch (Exception e)
+        {
+            _options.LogError(e, "Failed to capture screenshot.");
+            Complete(screenshot, null);
+        }
+    }
 
-            if (_options.BeforeCaptureScreenshotInternal?.Invoke(@event) is false)
+    private void Complete(TaskCompletionSource<byte[]?> screenshot, byte[]? result)
+    {
+        lock (_lock)
+        {
+            if (_pending == screenshot)
             {
-                yield break;
+                _pending = null;
             }
+        }
 
+        screenshot.TrySetResult(result);
+    }
+
+    private byte[]? CaptureScreenshot(SentryEvent @event)
+    {
+        Texture2D? screenshot = null;
+        try
+        {
             screenshot = CreateNewScreenshotTexture2D(_options);
 
             if (_options.BeforeSendScreenshotInternal != null)
@@ -67,7 +149,7 @@ internal IEnumerator CaptureScreenshotCoroutine(SentryEvent @event)
                 if (modifiedScreenshot == null)
                 {
                     _options.LogInfo("Screenshot discarded by BeforeSendScreenshot callback.");
-                    yield break;
+                    return null;
                 }
 
                 // Clean up - If the user returned a new texture object and did not modify the passed in one
@@ -79,44 +161,23 @@ internal IEnumerator CaptureScreenshotCoroutine(SentryEvent @event)
                 }
             }
 
-            var screenshotBytes = screenshot.EncodeToJPG(_options.ScreenshotCompression);
-            if (screenshotBytes is null || screenshotBytes.Length == 0)
-            {
-                _options.LogWarning("Screenshot capture returned empty data for event {0}", @event.EventId);
-                yield break;
-            }
-
-            var attachment = new SentryAttachment(
-                    AttachmentType.Default,
-                    new ByteAttachmentContent(screenshotBytes),
-                    "screenshot.jpg",
-                    "image/jpeg");
-
-            _options.LogDebug("Screenshot captured for event {0}", @event.EventId);
-
-            CaptureAttachment(@event.EventId, attachment);
-        }
-        catch (Exception e)
-        {
-            _options.LogError(e, "Failed to capture screenshot.");
+            return screenshot.EncodeToJPG(_options.ScreenshotCompression);
         }
         finally
         {
-            Interlocked.Exchange(ref _isCapturingScreenshot, 0);
-
             if (screenshot != null)
             {
                 UnityEngine.Object.Destroy(screenshot);
             }
         }
     }
 
+    internal virtual bool CanReadBackAsync()
+        => SystemInfo.supportsAsyncGPUReadback && _options.BackgroundWorker is not WebBackgroundWorker;
+
     internal virtual Texture2D CreateNewScreenshotTexture2D(SentryUnityOptions options)
         => SentryScreenshot.CreateNewScreenshotTexture2D(options);
 
-    internal virtual void CaptureAttachment(SentryId eventId, SentryAttachment attachment)
-        => (Sentry.SentrySdk.CurrentHub as Hub)?.CaptureAttachment(eventId, attachment);
-
-    internal virtual YieldInstruction WaitForEndOfFrame()
+    internal virtual YieldInstruction? WaitForEndOfFrame()
         => new WaitForEndOfFrame();
 }

src/Sentry.Unity/SentryMonoBehaviour.cs

@@ -59,7 +59,7 @@ public void StartAwakeSpan(MonoBehaviour monoBehaviour)
 /// </summary>
 public partial class SentryMonoBehaviour
 {
-    // Unbounded queue - used by the ScreenshotEventProcessor where capture is already limited by Interlocked to 1/frame
+    // Unbounded queue - used by the ScreenshotEventProcessor, which shares a capture among events until it is done
     private readonly ConcurrentQueue<IEnumerator> _coroutineQueue = new();
 
     public void QueueCoroutine(IEnumerator routine)

src/Sentry.Unity/SentryScreenshot.cs

@@ -1,5 +1,9 @@
+using System;
+using System.Threading.Tasks;
 using Sentry.Extensibility;
 using UnityEngine;
+using UnityEngine.Experimental.Rendering;
+using UnityEngine.Rendering;
 
 namespace Sentry.Unity;
 
@@ -21,6 +25,76 @@ public static Texture2D CreateNewScreenshotTexture2D(SentryUnityOptions options)
 
     // For testing
     internal static Texture2D CreateNewScreenshotTexture2D(SentryUnityOptions options, int width, int height)
+    {
+        var renderTexture = CaptureIntoRenderTexture(options, width, height);
+        var previousRenderTexture = RenderTexture.active;
+
+        try
+        {
+            // Captures the current screenshot synchronously.
+            var screenshot = new Texture2D(renderTexture.width, renderTexture.height, TextureFormat.RGB24, false);
+            RenderTexture.active = renderTexture;
+            screenshot.ReadPixels(new Rect(0, 0, renderTexture.width, renderTexture.height), 0, 0);
+            screenshot.Apply();
+
+            options.LogDebug("Screenshot captured at {0}x{1}.", renderTexture.width, renderTexture.height);
+
+            return screenshot;
+        }
+        finally
+        {
+            RenderTexture.active = previousRenderTexture;
+            RenderTexture.ReleaseTemporary(renderTexture);
+        }
+    }
+
+    /// <summary>
+    /// Reads the current screenshot back from the GPU and encodes it to JPG on a background thread, without stalling
+    /// the main thread.
+    /// </summary>
+    internal static void CaptureAsync(SentryUnityOptions options, Action<byte[]?> onCaptured)
+    {
+        var renderTexture = CaptureIntoRenderTexture(options, Screen.width, Screen.height);
+        var (width, height) = ((uint)renderTexture.width, (uint)renderTexture.height);
+
+        try
+        {
+            // The readback copies the texture as of now, so it can go back to the pool right away
+            AsyncGPUReadback.Request(renderTexture, 0, TextureFormat.RGBA32, readback =>
+            {
+                if (readback.hasError)
+                {
+                    options.LogError("Failed to read back the screenshot.");
+                    onCaptured(null);
+                    return;
+                }
+
+                var pixels = readback.GetData<byte>().ToArray();
+                Task.Run(() =>
+                {
+                    byte[]? bytes = null;
+                    try
+                    {
+                        bytes = ImageConversion.EncodeArrayToJPG(pixels, GraphicsFormat.R8G8B8A8_UNorm, width, height,
+                            0, options.ScreenshotCompression);
+                    }
+                    catch (Exception e)
+                    {
+                        options.LogError(e, "Failed to encode the screenshot.");
+                    }
+
+                    onCaptured(bytes);
+                });
+            });
+        }
+        finally
+        {
+            RenderTexture.ReleaseTemporary(renderTexture);
+        }
+    }
+
+    // Returns a temporary render texture holding the screenshot, scaled down to the configured quality
+    private static RenderTexture CaptureIntoRenderTexture(SentryUnityOptions options, int width, int height)
     {
         // Make sure the screenshot size does not exceed the target size by scaling the image while conserving the
         // original ratio based on which, width or height, is the smaller
@@ -37,17 +111,11 @@ internal static Texture2D CreateNewScreenshotTexture2D(SentryUnityOptions option
             }
         }
 
-        RenderTexture? renderTextureFull = null;
-        RenderTexture? renderTextureResized = null;
-        var previousRenderTexture = RenderTexture.active;
-
+        var renderTextureFull = RenderTexture.GetTemporary(Screen.width, Screen.height);
         try
         {
-            // Captures the current screenshot synchronously.
-            var screenshot = new Texture2D(width, height, TextureFormat.RGB24, false);
-            renderTextureFull = RenderTexture.GetTemporary(Screen.width, Screen.height);
             ScreenCapture.CaptureScreenshotIntoRenderTexture(renderTextureFull);
-            renderTextureResized = RenderTexture.GetTemporary(width, height);
+            var renderTextureResized = RenderTexture.GetTemporary(width, height);
 
             // The image may be mirrored on some platforms - mirror it back.
             // See https://docs.unity3d.com/2019.4/Documentation/Manual/SL-PlatformDifferences.html for more info.
@@ -62,27 +130,11 @@ internal static Texture2D CreateNewScreenshotTexture2D(SentryUnityOptions option
                 Graphics.Blit(renderTextureFull, renderTextureResized);
             }
 
-            RenderTexture.active = renderTextureResized;
-            screenshot.ReadPixels(new Rect(0, 0, width, height), 0, 0);
-            screenshot.Apply();
-
-            options.LogDebug("Screenshot captured at {0}x{1}.", width, height);
-
-            return screenshot;
+            return renderTextureResized;
         }
         finally
         {
-            RenderTexture.active = previousRenderTexture;
-
-            if (renderTextureFull)
-            {
-                RenderTexture.ReleaseTemporary(renderTextureFull);
-            }
-
-            if (renderTextureResized)
-            {
-                RenderTexture.ReleaseTemporary(renderTextureResized);
-            }
+            RenderTexture.ReleaseTemporary(renderTextureFull);
         }
     }
 }