Skip to content

Commit

Permalink
fix: connect exception to transaction (#1043)
Browse files Browse the repository at this point in the history
  • Loading branch information
bruno-garcia authored Jun 4, 2021
1 parent f60e3d7 commit fb47553
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 6 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
# Changelog

## Unreleased


### Fixes

- Connect middleware exceptions to transactions ([#1043](https://github.com/getsentry/sentry-dotnet/pull/1043))

## 3.4.0

### Features
Expand Down
23 changes: 20 additions & 3 deletions src/Sentry.AspNetCore/SentryTracingMiddleware.cs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Runtime.ExceptionServices;
using System.Threading.Tasks;
using Microsoft.AspNetCore.Builder;
using Microsoft.AspNetCore.Http;
Expand Down Expand Up @@ -122,10 +123,15 @@ public async Task InvokeAsync(HttpContext context)
scope.OnEvaluating += (_, _) => scope.Populate(context, _options);
});

Exception? exception = null;
try
{
await _next(context).ConfigureAwait(false);
}
catch (Exception e)
{
exception = e;
}
finally
{
if (transaction is not null)
Expand All @@ -146,9 +152,20 @@ public async Task InvokeAsync(HttpContext context)
transaction.Name = transactionName;
}

transaction.Finish(
SpanStatusConverter.FromHttpStatusCode(context.Response.StatusCode)
);
var status = SpanStatusConverter.FromHttpStatusCode(context.Response.StatusCode);
if (exception is null)
{
transaction.Finish(status);
}
else
{
transaction.Finish(exception, status);
}
}

if (exception is not null)
{
ExceptionDispatchInfo.Capture(exception).Throw();
}
}
}
Expand Down
7 changes: 4 additions & 3 deletions src/Sentry/Internal/Hub.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ internal class Hub : IHub, IDisposable
private readonly IDisposable _rootScope;
private readonly Enricher _enricher;

private readonly ConditionalWeakTable<Exception, ISpan> _exceptionToSpanMap = new();
// Internal for testability
internal ConditionalWeakTable<Exception, ISpan> ExceptionToSpanMap { get; } = new();

internal SentryScopeManager ScopeManager { get; }

Expand Down Expand Up @@ -144,7 +145,7 @@ public void BindException(Exception exception, ISpan span)
}

// Don't overwrite existing pair in the unlikely event that it already exists
_ = _exceptionToSpanMap.GetValue(exception, _ => span);
_ = ExceptionToSpanMap.GetValue(exception, _ => span);
}

public ISpan? GetSpan()
Expand All @@ -159,7 +160,7 @@ public void BindException(Exception exception, ISpan span)
{
// Find the span which is bound to the same exception
if (evt.Exception is { } exception &&
_exceptionToSpanMap.TryGetValue(exception, out var spanBoundToException))
ExceptionToSpanMap.TryGetValue(exception, out var spanBoundToException))
{
return spanBoundToException;
}
Expand Down
61 changes: 61 additions & 0 deletions test/Sentry.AspNetCore.Tests/SentryTracingMiddlewareTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -284,6 +284,67 @@ public async Task Transaction_sampling_context_contains_HTTP_context_data()
samplingContext.TryGetHttpRoute().Should().Be("/person/{id}");
samplingContext.TryGetHttpPath().Should().Be("/person/13");
}

[Fact]
public async Task Transaction_binds_exception_thrown()
{
// Arrange
TransactionSamplingContext samplingContext = null;

var sentryClient = Substitute.For<ISentryClient>();

var hub = new Internal.Hub(sentryClient, new SentryOptions
{
Dsn = DsnSamples.ValidDsnWithoutSecret,
TracesSampler = ctx =>
{
samplingContext = ctx;
return 1;
}
});
var exception = new Exception();

var server = new TestServer(new WebHostBuilder()
.UseDefaultServiceProvider(di => di.EnableValidation())
.UseSentry()
.ConfigureServices(services =>
{
services.AddRouting();
services.RemoveAll(typeof(Func<IHub>));
services.AddSingleton<Func<IHub>>(() => hub);
})
.Configure(app =>
{
app.UseRouting();
app.Use(async (r, c) =>
{
try
{
await c().ConfigureAwait(false);
}
catch
{
// We just want to know if it got into Sentry's Hub
}
});
app.UseSentryTracing();
app.UseEndpoints(routes =>
{
routes.Map("/person/{id}", _ => throw exception);
});
})
);

var client = server.CreateClient();

// Act
await client.GetAsync("/person/13");

// Assert
Assert.True(hub.ExceptionToSpanMap.TryGetValue(exception, out _));
}
}
}
#endif

0 comments on commit fb47553

Please sign in to comment.