SentryGraphQLHttpFailedRequestHandler improved issue grouping.

CHANGELOG.md

@@ -5,6 +5,7 @@
 ### Fixes
 
 - Captured [Http Client Errors](https://docs.sentry.io/platforms/dotnet/guides/aspnet/configuration/http-client-errors/) on .NET 5+ now include a full stack trace in order to improve Issue grouping ([#4724](https://github.com/getsentry/sentry-dotnet/pull/4724))
+- Captured [GraphQL Client Errors](https://docs.sentry.io/platforms/dotnet/configuration/graphql-client-errors/) on .NET 5+ now include a full stack trace in order to improve Issue grouping ([#4762](https://github.com/getsentry/sentry-dotnet/pull/4762))
 - Sentry Tracing middleware crashed ASP.NET Core in .NET 10 in 6.0.0-rc.1 and earlier ([#4747](https://github.com/getsentry/sentry-dotnet/pull/4747))
 
 ### Dependencies

src/Sentry/SentryGraphQLHttpFailedRequestHandler.cs

@@ -1,3 +1,4 @@
+using System.Runtime.ExceptionServices;
 using Sentry.Internal;
 using Sentry.Internal.Extensions;
 using Sentry.Protocol;
@@ -24,15 +25,24 @@ protected internal override void DoEnsureSuccessfulResponse([NotNull] HttpReques
         JsonElement? json = null;
         try
         {
-            json = GraphQLContentExtractor.ExtractResponseContentAsync(response, _options).Result;
+            json = GraphQLContentExtractor.ExtractResponseContentAsync(response, _options).GetAwaiter().GetResult();
             if (json is { } jsonElement)
             {
                 if (jsonElement.TryGetProperty("errors", out var errorsElement))
                 {
                     // We just show the first error... maybe there's a better way to do this when multiple errors exist.
                     // We should check what the Java code is doing.
                     var errorMessage = errorsElement[0].GetProperty("message").GetString() ?? "GraphQL Error";
-                    throw new GraphQLHttpRequestException(errorMessage);
+                    var exception = new GraphQLHttpRequestException(errorMessage);
+
+#if NET5_0_OR_GREATER
+                    // Add a full stack trace into the exception to improve Issue grouping,
+                    // see https://github.com/getsentry/sentry-dotnet/issues/3582
+                    ExceptionDispatchInfo.SetCurrentStackTrace(exception);
+#endif
+                    // Wrap in a new exception to preserve the stack trace when thrown and caught.
+                    // The inner exception will have the full stack trace for better issue grouping.
+                    throw new GraphQLHttpRequestException(errorMessage, exception);
                 }
             }
             // No GraphQL errors, but we still might have an HTTP error status

test/Sentry.Tests/SentryGraphQlHttpFailedRequestHandlerTests.cs

@@ -3,6 +3,9 @@ namespace Sentry.Tests;
 public class SentryGraphQlHttpFailedRequestHandlerTests
 {
     private const string ValidQuery = "query getAllNotes { notes { id } }";
+    private const string DefaultErrorMessage = "Bad query";
+    private const string DefaultErrorCode = "BAD_OP";
+
     private static HttpResponseMessage ForbiddenResponse()
         => new(HttpStatusCode.Forbidden);
 
@@ -12,7 +15,7 @@ private static HttpResponseMessage InternalServerErrorResponse()
     private HttpResponseMessage PreconditionFailedResponse()
         => new(HttpStatusCode.PreconditionFailed)
         {
-            Content = SentryGraphQlTestHelpers.ErrorContent("Bad query", "BAD_OP")
+            Content = SentryGraphQlTestHelpers.ErrorContent(DefaultErrorMessage, DefaultErrorCode)
         };
 
     [Fact]
@@ -81,7 +84,7 @@ public void HandleResponse_NoMatchingTarget_DontCapture()
     }
 
     [Fact]
-    public void HandleResponse_NoError_BaseCapturesFailedRequests()
+    public void HandleResponse_NoGraphQLError_HttpHandlerFallbackCapturesFailedRequests()
     {
         // Arrange
         var hub = Substitute.For<IHub>();
@@ -91,16 +94,26 @@ public void HandleResponse_NoError_BaseCapturesFailedRequests()
         };
         var sut = new SentryGraphQLHttpFailedRequestHandler(hub, options);
 
-        var response = InternalServerErrorResponse();
-        response.RequestMessage = new HttpRequestMessage();
+        // Response has valid JSON but no GraphQL errors - just HTTP error status
+        var response = new HttpResponseMessage(HttpStatusCode.InternalServerError)
+        {
+            Content = SentryGraphQlTestHelpers.JsonContent(new { data = "some response data" }),
+            RequestMessage = new HttpRequestMessage(HttpMethod.Post, "http://example.com/graphql")
+        };
 
         // Act
+        SentryEvent @event = null;
+        hub.CaptureEvent(Arg.Do<SentryEvent>(e => @event = e), hint: Arg.Any<SentryHint>());
         sut.HandleResponse(response);
 
         // Assert
         hub.Received(1).CaptureEvent(
             Arg.Any<SentryEvent>(),
             Arg.Any<Scope>(), Arg.Any<SentryHint>());
+
+        // Should fall back to HTTP handler, capturing HttpRequestException
+        @event.Exception.Should().BeOfType<HttpRequestException>();
+        @event.Exception!.Message.Should().NotBeNullOrWhiteSpace();
     }
 
     [Fact]
@@ -115,15 +128,21 @@ public void HandleResponse_Error_Capture()
         var sut = new SentryGraphQLHttpFailedRequestHandler(hub, options);
 
         var response = PreconditionFailedResponse();
-        response.RequestMessage = new HttpRequestMessage();
+        response.RequestMessage = SentryGraphQlTestHelpers.GetRequestQuery(ValidQuery);
 
         // Act
+        SentryEvent @event = null;
+        hub.CaptureEvent(Arg.Do<SentryEvent>(e => @event = e), hint: Arg.Any<SentryHint>());
         sut.HandleResponse(response);
 
         // Assert
         hub.Received(1).CaptureEvent(
             Arg.Any<SentryEvent>(),
             Arg.Any<Scope>(), Arg.Any<SentryHint>());
+
+        // Verify it's actually a GraphQL error, not HTTP error fallback
+        @event.Exception.Should().BeOfType<GraphQLHttpRequestException>();
+        @event.Exception!.Message.Should().Be(DefaultErrorMessage);
     }
 
     [Fact]
@@ -139,14 +158,15 @@ public void HandleResponse_Error_DontSendPii()
 
         var response = PreconditionFailedResponse();
         var uri = new Uri("http://admin:1234@localhost/test/path?query=string#fragment");
-        response.RequestMessage = new HttpRequestMessage(HttpMethod.Get, uri);
+        response.RequestMessage = SentryGraphQlTestHelpers.GetRequestQuery(ValidQuery, uri.ToString());
 
         // Act
         SentryEvent @event = null;
         hub.CaptureEvent(Arg.Do<SentryEvent>(e => @event = e), hint: Arg.Any<SentryHint>());
         sut.HandleResponse(response);
 
         // Assert
+        @event.Exception.Should().BeOfType<GraphQLHttpRequestException>();
         @event.Request.Url.Should().Be("http://localhost/test/path?query=string"); // No admin:1234
         @event.Request.Data.Should().BeNull();
         var responseContext = @event.Contexts[Response.Type] as Response;
@@ -188,6 +208,13 @@ public void HandleResponse_Error_CaptureRequestAndResponse()
         {
             @event.Should().NotBeNull();
 
+            // Ensure it's a GraphQL exception (not HTTP fallback)
+            @event.Exception.Should().BeOfType<GraphQLHttpRequestException>();
+            @event.Exception!.Message.Should().Be(DefaultErrorMessage);
+            @event.Exception!.InnerException.Should().NotBeNull("inner exception should have the stack trace");
+            @event.Exception!.InnerException.Should().BeOfType<GraphQLHttpRequestException>();
+            @event.Exception!.InnerException!.Message.Should().Be(DefaultErrorMessage);
+
             // Ensure the mechanism is set
             @event.Exception?.Data[Mechanism.MechanismKey].Should().Be(SentryGraphQLHttpFailedRequestHandler.MechanismType);
             @event.Exception?.Data[Mechanism.HandledKey].Should().Be(false);
@@ -205,7 +232,7 @@ public void HandleResponse_Error_CaptureRequestAndResponse()
             responseContext?.StatusCode.Should().Be((short)response.StatusCode);
             responseContext?.BodySize.Should().Be(response.Content.Headers.ContentLength);
             responseContext?.Data?.ToString().Should().Be(
-                SentryGraphQlTestHelpers.ErrorContent("Bad query", "BAD_OP").ReadAsJson().ToString()
+                SentryGraphQlTestHelpers.ErrorContent(DefaultErrorMessage, DefaultErrorCode).ReadAsJson().ToString()
                 );
 
             @event.Contexts.Response.Headers.Should().ContainKey("myHeader");
@@ -249,4 +276,65 @@ public void HandleResponse_Error_ResponseAsHint()
             hint.Items[HintTypes.HttpResponseMessage].Should().Be(response);
         }
     }
+
+    [Fact]
+    public void HandleResponse_GraphQLError_HasExceptionWithStackTrace()
+    {
+        // Arrange
+        var hub = Substitute.For<IHub>();
+        var options = new SentryOptions
+        {
+            CaptureFailedRequests = true
+        };
+        var sut = new SentryGraphQLHttpFailedRequestHandler(hub, options);
+
+        var response = PreconditionFailedResponse();
+        response.RequestMessage = SentryGraphQlTestHelpers.GetRequestQuery(ValidQuery);
+
+        // Act
+        SentryEvent @event = null;
+        hub.CaptureEvent(Arg.Do<SentryEvent>(e => @event = e), hint: Arg.Any<SentryHint>());
+        sut.HandleResponse(response);
+
+        // Assert
+        using (new AssertionScope())
+        {
+            @event.Should().NotBeNull();
+            @event.Exception.Should().NotBeNull();
+            @event.Exception!.StackTrace.Should().NotBeNullOrWhiteSpace();
+        }
+    }
+
+#if NET5_0_OR_GREATER // This test is only valid on .NET 5+ where we can use SetCurrentStackTrace
+    [Fact]
+    public void HandleResponse_GraphQLError_ExceptionStackTraceHasCallerContext()
+    {
+        // Arrange
+        var hub = Substitute.For<IHub>();
+        var options = new SentryOptions
+        {
+            CaptureFailedRequests = true
+        };
+        var sut = new SentryGraphQLHttpFailedRequestHandler(hub, options);
+
+        var response = PreconditionFailedResponse();
+        response.RequestMessage = SentryGraphQlTestHelpers.GetRequestQuery(ValidQuery);
+
+        // Act
+        SentryEvent @event = null;
+        hub.CaptureEvent(Arg.Do<SentryEvent>(e => @event = e), hint: Arg.Any<SentryHint>());
+        sut.HandleResponse(response);
+
+        // Assert
+        using (new AssertionScope())
+        {
+            @event.Should().NotBeNull();
+            @event.Exception.Should().NotBeNull();
+            @event.Exception!.InnerException.Should().NotBeNull();
+
+            // Inner exception's stack trace should include this test method name, proving we captured caller context on .NET 5+
+            @event.Exception!.InnerException!.StackTrace.Should().Contain(nameof(HandleResponse_GraphQLError_ExceptionStackTraceHasCallerContext));
+        }
+    }
+#endif
 }

test/Sentry.Tests/SentryGraphQlTestHelpers.cs

@@ -46,17 +46,20 @@ public static StringContent ResponesContent(string responseText) => JsonContent(
 
     /// <summary>
     /// e.g.
-    /// "[{"message":"Query does not contain operation \u0027getAllNotes\u0027.","extensions":{"code":"INVALID_OPERATION","codes":["INVALID_OPERATION"]}}]"
+    /// "{"errors": [{"message":"Query does not contain operation \u0027getAllNotes\u0027.","extensions":{"code":"INVALID_OPERATION","codes":["INVALID_OPERATION"]}}]}"
     /// </summary>
     public static StringContent ErrorContent(string errorMessage, string errorCode) => JsonContent(
-        new dynamic[]
+        new
         {
-            new
+            errors = new dynamic[]
             {
-                message = errorMessage,
-                extensions = new {
-                    code = errorCode,
-                    codes = new dynamic[]{ errorCode }
+                new
+                {
+                    message = errorMessage,
+                    extensions = new {
+                        code = errorCode,
+                        codes = new dynamic[]{ errorCode }
+                    }
                 }
             }
         }