From dd0359baf09373ce845e68549d60b32468dc2dac Mon Sep 17 00:00:00 2001 From: Eric Sampson Date: Sat, 22 Nov 2025 09:54:49 -0600 Subject: [PATCH 1/2] Improve the issue grouping of the SentryGraphQLHttpFailedRequestHandler --- .../SentryGraphQLHttpFailedRequestHandler.cs | 14 ++- ...tryGraphQlHttpFailedRequestHandlerTests.cs | 102 ++++++++++++++++-- test/Sentry.Tests/SentryGraphQlTestHelpers.cs | 17 +-- 3 files changed, 117 insertions(+), 16 deletions(-) diff --git a/src/Sentry/SentryGraphQLHttpFailedRequestHandler.cs b/src/Sentry/SentryGraphQLHttpFailedRequestHandler.cs index a40a5c6383..29f50c077e 100644 --- a/src/Sentry/SentryGraphQLHttpFailedRequestHandler.cs +++ b/src/Sentry/SentryGraphQLHttpFailedRequestHandler.cs @@ -1,3 +1,4 @@ +using System.Runtime.ExceptionServices; using Sentry.Internal; using Sentry.Internal.Extensions; using Sentry.Protocol; @@ -24,7 +25,7 @@ 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)) @@ -32,7 +33,16 @@ protected internal override void DoEnsureSuccessfulResponse([NotNull] HttpReques // 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 diff --git a/test/Sentry.Tests/SentryGraphQlHttpFailedRequestHandlerTests.cs b/test/Sentry.Tests/SentryGraphQlHttpFailedRequestHandlerTests.cs index 0b361d2e29..92d48de597 100644 --- a/test/Sentry.Tests/SentryGraphQlHttpFailedRequestHandlerTests.cs +++ b/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(); @@ -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(e => @event = e), hint: Arg.Any()); sut.HandleResponse(response); // Assert hub.Received(1).CaptureEvent( Arg.Any(), Arg.Any(), Arg.Any()); + + // Should fall back to HTTP handler, capturing HttpRequestException + @event.Exception.Should().BeOfType(); + @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(e => @event = e), hint: Arg.Any()); sut.HandleResponse(response); // Assert hub.Received(1).CaptureEvent( Arg.Any(), Arg.Any(), Arg.Any()); + + // Verify it's actually a GraphQL error, not HTTP error fallback + @event.Exception.Should().BeOfType(); + @event.Exception!.Message.Should().Be(DefaultErrorMessage); } [Fact] @@ -139,7 +158,7 @@ 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; @@ -147,6 +166,7 @@ public void HandleResponse_Error_DontSendPii() sut.HandleResponse(response); // Assert + @event.Exception.Should().BeOfType(); @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(); + @event.Exception!.Message.Should().Be(DefaultErrorMessage); + @event.Exception!.InnerException.Should().NotBeNull("inner exception should have the stack trace"); + @event.Exception!.InnerException.Should().BeOfType(); + @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(); + 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(e => @event = e), hint: Arg.Any()); + 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(); + 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(e => @event = e), hint: Arg.Any()); + 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 } diff --git a/test/Sentry.Tests/SentryGraphQlTestHelpers.cs b/test/Sentry.Tests/SentryGraphQlTestHelpers.cs index cb761f32b4..0ea7f6d255 100644 --- a/test/Sentry.Tests/SentryGraphQlTestHelpers.cs +++ b/test/Sentry.Tests/SentryGraphQlTestHelpers.cs @@ -46,17 +46,20 @@ public static StringContent ResponesContent(string responseText) => JsonContent( /// /// 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"]}}]}" /// 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 } + } } } } From e7cd5df5e9fd2adbd141562b0c9a3da9f9537d00 Mon Sep 17 00:00:00 2001 From: Eric Sampson Date: Sat, 22 Nov 2025 09:59:52 -0600 Subject: [PATCH 2/2] changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 67608bc13f..bc7b2ebaa4 100644 --- a/CHANGELOG.md +++ b/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