Skip to content
Open
Show file tree
Hide file tree
Changes from all 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.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);
Copy link

Choose a reason for hiding this comment

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

Bug: Inner exception missing stack trace on pre-.NET 5

On pre-.NET 5 platforms, the inner exception won't have a stack trace because it's never thrown and SetCurrentStackTrace isn't available. The wrapper exception at line 45 only captures a minimal stack trace from the immediate throw/catch within the same method. To match the HTTP handler pattern, the inner exception should be thrown and caught on pre-.NET 5 platforms before wrapping, similar to the try-catch pattern used in SentryHttpFailedRequestHandler.

Fix in Cursor Fix in Web

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's fine.

Copy link
Collaborator

Choose a reason for hiding this comment

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

There's something weird about wrapping an exception that we just created (on line 36) in another identical exception.

Could we do this the same way as the other handler:

#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);
#else
// Where SetRemoteStackTrace is not available, throw and catch to get a basic stack trace
try
{
throw exception;
}
catch (HttpRequestException ex)
{
exception = ex;
}
#endif

}
}
// No GraphQL errors, but we still might have an HTTP error status
Expand Down
102 changes: 95 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,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);
Expand All @@ -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");
Expand Down Expand Up @@ -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
}
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