Failed requests add a Hint for the HttpResponseMessage

src/Sentry/Hint.cs

@@ -6,18 +6,23 @@
 namespace Sentry;
 
 /// <summary>
-/// A hint for the <see cref="SentryClient"/> to decide whether an event should be sent or cached. It also
-/// holds data that should be injected into the event.
+/// A hint that can be provided when capturing a <see cref="SentryEvent"/> or adding a <see cref="Breadcrumb"/>.
+/// Hints can be used to filter or modify events or breadcrumbs before they are sent to Sentry.
 /// </summary>
 public class Hint : ICollection, IEnumerable<KeyValuePair<string, object?>>
 {
     private readonly Dictionary<string, object?> _internalStorage = new();
     private readonly List<Attachment> _attachments = new();
 
-    public object? this[string name]
+    /// <summary>
+    /// Gets or sets additional values to be provided with the hint
+    /// </summary>
+    /// <param name="key">The key</param>
+    /// <returns>The value with the specified key or null if none exist.</returns>
+    public object? this[string key]
     {
-        get => _internalStorage.GetValueOrDefault(name);
-        set => _internalStorage[name] = value;
+        get => _internalStorage.GetValueOrDefault(key);
+        set => _internalStorage[key] = value;
     }
 
     internal void AddAttachmentsInternal(IEnumerable<Attachment> attachments)
@@ -28,35 +33,75 @@ internal void AddAttachmentsInternal(IEnumerable<Attachment> attachments)
         }
     }
 
+    /// <summary>
+    /// Adds one or more attachments to the Hint.
+    /// </summary>
+    /// <param name="attachments"></param>
     public void AddAttachments(params Attachment[] attachments) => AddAttachmentsInternal(attachments);
 
-    public void AddAttachments(ICollection<Attachment> attachments) => AddAttachmentsInternal(attachments);
+    /// <summary>
+    /// Adds multiple attachments to the Hint.
+    /// </summary>
+    /// <param name="attachments"></param>
+    public void AddAttachments(IEnumerable<Attachment> attachments) => AddAttachmentsInternal(attachments);
 
+    /// <summary>
+    /// Attachments added to the Hint.
+    /// </summary>
     public ICollection<Attachment> Attachments => _attachments;
 
+    /// <summary>
+    /// Clears any values stored in <see cref="this[string]"/>
+    /// </summary>
     public void Clear() => _internalStorage.Clear();
 
+    /// <summary>
+    /// Checks if the specified key exists
+    /// </summary>
+    /// <param name="key">The key</param>
+    /// <returns>True if the key exists. False otherwise.</returns>
     public bool ContainsKey(string key) => _internalStorage.ContainsKey(key);
 
+    /// <inheritdoc />
     public void CopyTo(Array array, int index) => ((ICollection)_internalStorage).CopyTo(array, index);
 
+    /// <inheritdoc />
     public int Count => _internalStorage.Count;
 
     IEnumerator IEnumerable.GetEnumerator() => _internalStorage.GetEnumerator();
 
+    /// <inheritdoc />
     public IEnumerator<KeyValuePair<string, object?>> GetEnumerator()
         => ((IEnumerable<KeyValuePair<string, object?>>)_internalStorage).GetEnumerator();
 
-    public T? GetAs<T>(string name) where T : class? => (this[name] is T typedHintValue) ? typedHintValue : null;
+    /// <summary>
+    /// Gets the value with the specified key as type <typeparamref name="T"/>
+    /// </summary>
+    /// <typeparam name="T">They expected value type</typeparam>
+    /// <param name="key">The key</param>
+    /// <returns>A value of type <typeparamref name="T"/> if one exists with the specified key or null otherwise.</returns>
+    public T? GetValue<T>(string key) where T : class? => (this[key] is T typedHintValue) ? typedHintValue : null;
 
+    /// <inheritdoc />
     public bool IsSynchronized => ((ICollection)_internalStorage).IsSynchronized;
 
-    public void Remove(string name) => _internalStorage.Remove(name);
+    /// <summary>
+    /// Remves the value with the specified key
+    /// </summary>
+    /// <param name="key"></param>
+    public void Remove(string key) => _internalStorage.Remove(key);
 
+    /// <summary>
+    /// Gets or sets a Screenshot for the Hint
+    /// </summary>
     public Attachment? Screenshot { get; set; }
 
+    /// <inheritdoc />
     public object SyncRoot => ((ICollection)_internalStorage).SyncRoot;
 
+    /// <summary>
+    /// Gets or sets a ViewHierarchy for the Hint
+    /// </summary>
     public Attachment? ViewHierarchy { get; set; }
 
     /// <summary>

src/Sentry/HintTypes.cs

@@ -0,0 +1,12 @@
+namespace Sentry;
+
+/// <summary>
+/// Constants used to name Hints generated by the Sentry SDK
+/// </summary>
+public static class HintTypes
+{
+    /// <summary>
+    /// Used for HttpResponseMessage hints
+    /// </summary>
+    public const string HttpResponseMessage = "http-response-message";
+}

src/Sentry/SentryFailedRequestHandler.cs

@@ -75,6 +75,8 @@ public void HandleResponse(HttpResponseMessage response)
             exception.SetSentryMechanism(MechanismType);
 
             var @event = new SentryEvent(exception);
+            var hint = new Hint();
+            hint[HintTypes.HttpResponseMessage] = response;
 
             var sentryRequest = new Request
             {
@@ -103,7 +105,7 @@ public void HandleResponse(HttpResponseMessage response)
             @event.Request = sentryRequest;
             @event.Contexts[Response.Type] = responseContext;
 
-            _hub.CaptureEvent(@event);
+            _hub.CaptureEvent(@event, hint);
         }
     }
 }

src/Sentry/SentryOptions.cs

@@ -306,7 +306,7 @@ public float? SampleRate
 
     private Func<SentryEvent, Hint, SentryEvent?>? _beforeSend;
 
-    internal Func<SentryEvent, Hint, SentryEvent?>? BeforeSendInternal { get => _beforeSend; }
+    internal Func<SentryEvent, Hint, SentryEvent?>? BeforeSendInternal => _beforeSend;
 
     /// <summary>
     /// A callback to invoke before sending an event to Sentry

test/Sentry.Tests/HintTests.cs

@@ -3,11 +3,12 @@ namespace Sentry.Tests;
 public class HintTests
 {
     private Attachment FakeAttachment(string name = "test.txt")
-        => new Attachment(
+        => new(
             AttachmentType.Default,
             new StreamAttachmentContent(new MemoryStream(new byte[] { 1 })),
             name,
-            null);
+            null
+            );
 
     [Fact]
     public void AddAttachments_WithNullAttachments_DoesNothing()
@@ -172,27 +173,27 @@ public void Count_ReturnsCorrectValue_WhenHintHasItems()
     }
 
     [Fact]
-    public void GetAs_WithNonExistingName_ReturnsNull()
+    public void GetValue_WithNonExistingKey_ReturnsNull()
     {
         // Arrange
         var hint = new Hint();
 
         // Act
-        var result = hint.GetAs<string>("non-existing");
+        var result = hint.GetValue<string>("non-existing");
 
         // Assert
         Assert.Null(result);
     }
 
     [Fact]
-    public void GetAs_WithExistingName_ReturnsValue()
+    public void GetValue_WithExistingKey_ReturnsValue()
     {
         // Arrange
         var hint = new Hint();
         hint["key"] = "value";
 
         // Act
-        var result = hint.GetAs<string>("key");
+        var result = hint.GetValue<string>("key");
 
         // Assert
         Assert.Equal("value", result);
@@ -219,7 +220,7 @@ public void GetEnumerator_ReturnsValidEnumreator()
     }
 
     [Fact]
-    public void Remove_WithExistingName_RemovesEntry()
+    public void Remove_WithExistingKey_RemovesEntry()
     {
         // Arrange
         var hint = new Hint();

test/Sentry.Tests/Protocol/TransactionTests.cs

@@ -296,11 +296,15 @@ public void Finish_LinksExceptionToEvent()
         // Assert
         transaction.Status.Should().Be(SpanStatus.InternalError);
 
-        client.Received(1).CaptureEvent(Arg.Is<SentryEvent>(e =>
-            e.Contexts.Trace.TraceId == transaction.TraceId &&
-            e.Contexts.Trace.SpanId == transaction.SpanId &&
-            e.Contexts.Trace.ParentSpanId == transaction.ParentSpanId
-        ), Arg.Any<Scope>());
+        client.Received(1).CaptureEvent(
+            Arg.Is<SentryEvent>(e =>
+                e.Contexts.Trace.TraceId == transaction.TraceId &&
+                e.Contexts.Trace.SpanId == transaction.SpanId &&
+                e.Contexts.Trace.ParentSpanId == transaction.ParentSpanId
+            ),
+            Arg.Any<Hint>(),
+            Arg.Any<Scope>()
+            );
     }
 
     [Fact]

test/Sentry.Tests/SentryFailedRequestHandlerTests.cs

@@ -119,7 +119,11 @@ public void HandleResponse_Capture_FailedRequest()
         sut.HandleResponse(response);
 
         // Assert
-        _hub.Received(1).CaptureEvent(Arg.Any<SentryEvent>(), Arg.Any<Scope>());
+        _hub.Received(1).CaptureEvent(
+            Arg.Any<SentryEvent>(),
+            Arg.Any<Hint>(),
+            Arg.Any<Scope>()
+            );
     }
 
     [Fact]
@@ -145,7 +149,10 @@ public void HandleResponse_Capture_RequestAndResponse()
 
         // Act
         SentryEvent @event = null;
-        _hub.CaptureEvent(Arg.Do<SentryEvent>(e => @event = e));
+        _hub.CaptureEvent(
+            Arg.Do<SentryEvent>(e => @event = e),
+            Arg.Any<Hint>()
+            );
         sut.HandleResponse(response);
 
         // Assert
@@ -192,7 +199,10 @@ public void HandleResponse_Capture_Default_SkipCookiesAndHeaders()
 
         // Act
         SentryEvent @event = null;
-        _hub.CaptureEvent(Arg.Do<SentryEvent>(e => @event = e));
+        _hub.CaptureEvent(
+            Arg.Do<SentryEvent>(e => @event = e),
+            Arg.Any<Hint>()
+            );
         sut.HandleResponse(response);
 
         // Assert
@@ -205,4 +215,35 @@ public void HandleResponse_Capture_Default_SkipCookiesAndHeaders()
             @event.Contexts.Response.Cookies.Should().BeNullOrEmpty();
         }
     }
+
+    [Fact]
+    public void HandleResponse_Hint_Response()
+    {
+        // Arrange
+        var options = new SentryOptions
+        {
+            CaptureFailedRequests = true
+        };
+        var sut = GetSut(options);
+
+        var response = InternalServerErrorResponse(); // This is in the range
+        response.RequestMessage = new HttpRequestMessage(HttpMethod.Post, "http://foo/bar");
+
+        // Act
+        Hint hint = null;
+        _hub.CaptureEvent(
+            Arg.Any<SentryEvent>(),
+            Arg.Do<Hint>(h => hint = h)
+            );
+        sut.HandleResponse(response);
+
+        // Assert
+        using (new AssertionScope())
+        {
+            hint.Should().NotBeNull();
+
+            // Response should be captured
+            hint[HintTypes.HttpResponseMessage].Should().Be(response);
+        }
+    }
 }

test/Sentry.Tests/SentrySdkTests.cs

@@ -114,21 +114,15 @@ public void Init_InvalidDsnEnvironmentVariable_Throws()
     {
         // If the variable was set, to non empty string but value is broken, better crash than silently disable
         var ex = Assert.Throws<ArgumentException>(() =>
-            SentrySdk.Init(o =>
-            {
-                o.FakeSettings().EnvironmentVariables[DsnEnvironmentVariable] = InvalidDsn;
-            }));
+            SentrySdk.Init(o => o.FakeSettings().EnvironmentVariables[DsnEnvironmentVariable] = InvalidDsn));
 
         Assert.Equal("Invalid DSN: A Project Id is required.", ex.Message);
     }
 
     [Fact]
     public void Init_DisableDsnEnvironmentVariable_DisablesSdk()
     {
-        using var _ = SentrySdk.Init(o =>
-        {
-            o.FakeSettings().EnvironmentVariables[DsnEnvironmentVariable] = Constants.DisableSdkDsnValue;
-        });
+        using var _ = SentrySdk.Init(o => o.FakeSettings().EnvironmentVariables[DsnEnvironmentVariable] = Constants.DisableSdkDsnValue);
 
         Assert.False(SentrySdk.IsEnabled);
     }