From 997a310547145bb43d73807e0a13be2fae643a22 Mon Sep 17 00:00:00 2001 From: Bruno Garcia Date: Fri, 10 Sep 2021 18:11:03 -0400 Subject: [PATCH] fix: unhandled error crashed session (#1193) --- CHANGELOG.md | 6 ++++++ src/Sentry/Internal/Hub.cs | 6 ++---- src/Sentry/SentryEvent.cs | 7 +++++++ test/Sentry.Tests/HubTests.cs | 33 +++++++++++++++++++++++++++++++++ 4 files changed, 48 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index eee630a9a1..c13cfb109d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Exceptions from UnhandledExceptionIntegration were not marking sessions as crashed ([#1193](https://github.com/getsentry/sentry-dotnet/pull/1193)) + ## 3.9.1 ### Fixes diff --git a/src/Sentry/Internal/Hub.cs b/src/Sentry/Internal/Hub.cs index 2b83c2b598..91a7a49ba8 100644 --- a/src/Sentry/Internal/Hub.cs +++ b/src/Sentry/Internal/Hub.cs @@ -304,15 +304,13 @@ public SentryId CaptureEvent(SentryEvent evt, Scope? scope = null) actualScope.SessionUpdate = evt switch { - // TODO: Extract both branches as internal extension methods (IsCrashed and IsErrored): - // Event contains a terminal exception -> end session as crashed - var e when e.SentryExceptions?.Any(x => !(x.Mechanism?.Handled ?? true)) ?? false => + var e when e.HasUnhandledException => _sessionManager.EndSession(SessionEndStatus.Crashed), // Event contains a non-terminal exception -> report error // (this might return null if the session has already reported errors before) - var e when e.Exception is not null || e.SentryExceptions?.Any() == true => + var e when e.HasException => _sessionManager.ReportError(), // Event doesn't contain any kind of exception -> no reason to attach session update diff --git a/src/Sentry/SentryEvent.cs b/src/Sentry/SentryEvent.cs index c4cba87fa0..4d79fe2d6e 100644 --- a/src/Sentry/SentryEvent.cs +++ b/src/Sentry/SentryEvent.cs @@ -164,6 +164,13 @@ public IReadOnlyList Fingerprint /// public IReadOnlyDictionary Tags => _tags ??= new Dictionary(); + internal bool HasException => Exception is not null || SentryExceptions?.Any() == true; + + internal bool HasUnhandledException => (SentryExceptions?.Any(e => !(e.Mechanism?.Handled ?? true)) ?? false) + // Before event is processed by the client and SentryExceptions created. + // See: AppDomainUnhandledExceptionIntegration + || Exception?.Data[Mechanism.HandledKey] is false; + /// /// Creates a new instance of . /// diff --git a/test/Sentry.Tests/HubTests.cs b/test/Sentry.Tests/HubTests.cs index 6e565292ab..b0a65e48d1 100644 --- a/test/Sentry.Tests/HubTests.cs +++ b/test/Sentry.Tests/HubTests.cs @@ -7,6 +7,7 @@ using Sentry; using Sentry.Extensibility; using Sentry.Infrastructure; +using Sentry.Integrations; using Sentry.Internal; using Sentry.Protocol; using Sentry.Protocol.Envelopes; @@ -350,6 +351,38 @@ public void CaptureEvent_ActiveSession_UnhandledExceptionSessionEndedAsCrashed() )); } + [Fact] + public void AppDomainUnhandledExceptionIntegration_ActiveSession_UnhandledExceptionSessionEndedAsCrashed() + { + // Arrange + var worker = Substitute.For(); + + var options = new SentryOptions { Dsn = DsnSamples.ValidDsnWithSecret }; + var client = new SentryClient(options, worker); + var hub = new Hub(options, client); + + var integration = new AppDomainUnhandledExceptionIntegration(Substitute.For()); + integration.Register(hub, options); + + hub.StartSession(); + + // Act + // Simulate a terminating exception + integration.Handle(this, new UnhandledExceptionEventArgs(new Exception("test"), true)); + + // Assert + worker.Received().EnqueueEnvelope( + Arg.Is(e => + e.Items + .Select(i => i.Payload) + .OfType() + .Select(i => i.Source) + .OfType() + .Single() + .EndStatus == SessionEndStatus.Crashed + )); + } + [Fact] public void StartTransaction_NameOpDescription_Works() {