Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
14 changes: 12 additions & 2 deletions src/Sentry/SentryGraphQLHttpFailedRequestHandler.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Runtime.ExceptionServices;
using Sentry.Internal;
using Sentry.Internal.Extensions;
using Sentry.Protocol;
Expand All @@ -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.Throw(ExceptionDispatchInfo.SetCurrentStackTrace(exception));
#else
// Where SetCurrentStackTrace is not available, just throw the Exception
throw exception;
#endif

This comment was marked as outdated.

Copy link
Member

@Flash0ver Flash0ver Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm ... correct ... but

in SentryHttpFailedRequestHandler we essentially only read the Status-Code

here, in SentryGraphQLHttpFailedRequestHandler, we also try to extract JSON content from the HttpResponseMessage ... I'm a bit uncertain whether this will always succeed ... should the JSON Content from the Response be malformed then GraphQLContentExtractor.ExtractResponseContentAsync would throw a JsonReaderException

I guess this would be another issue/changeset to refactor this approach,
as this PR is not changing anything about the throw-behavior of this method,
but instead replaced the Stack-Trace of the Exception with a richer Stack-Trace for more detailed grouping in Sentry.
Technically, I do agree that this would be nice to have, to make the NET5_0_OR_GREATER path "throw-free".

@jamescrosswell what do you think?

Copy link
Contributor Author

@ericsampson ericsampson Nov 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For context I wanted to keep the changes targeted and minimal vs the current strategy (at least for a first PR), I figured it's safer to have the outer try as in the original to capture json extraction failures--that seems like "not an edge case".

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems unrelated to this PR yeah... I'm not sure I see a problem. If the JSON is malformed, that would be caught and converted to a sentry event here, which I don't think is unexpected behaviour. Is there a better alternative?

}
}
// No GraphQL errors, but we still might have an HTTP error status
Expand Down
98 changes: 91 additions & 7 deletions test/Sentry.Tests/SentryGraphQlHttpFailedRequestHandlerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand All @@ -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]
Expand Down Expand Up @@ -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>();
Expand All @@ -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]
Expand All @@ -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]
Expand All @@ -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;
Expand Down Expand Up @@ -188,6 +208,10 @@ 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);

// Ensure the mechanism is set
@event.Exception?.Data[Mechanism.MechanismKey].Should().Be(SentryGraphQLHttpFailedRequestHandler.MechanismType);
@event.Exception?.Data[Mechanism.HandledKey].Should().Be(false);
Expand All @@ -205,7 +229,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");
Expand Down Expand Up @@ -249,4 +273,64 @@ 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();

// Exception's stack trace should include this test method name, proving we captured caller context on .NET 5+
@event.Exception!.StackTrace.Should().Contain(nameof(HandleResponse_GraphQLError_ExceptionStackTraceHasCallerContext));
}
}
#endif
}
17 changes: 10 additions & 7 deletions test/Sentry.Tests/SentryGraphQlTestHelpers.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}
}
}
}
Expand Down
Loading