Skip to content

Commit a2ecbbc

Browse files
refactor: move handled before configureScope and guard disabled hub
Addresses review feedback on #5449: - reorder the scope-callback overloads to (ex, handled, configureScope) so a long lambda stays the last argument - skip the Exception.Data mutation when the hub is disabled, matching SentryClientExtensions.CaptureException - note that the CaptureExceptionInternal fallback is defensive only Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3663ee5 commit a2ecbbc

8 files changed

Lines changed: 39 additions & 18 deletions

src/Sentry/HubExtensions.cs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -271,8 +271,8 @@ public LockedScope(IHub hub)
271271

272272
internal static SentryId CaptureExceptionInternal(this IHub hub, Exception ex)
273273
{
274-
// Integrations stamp the flag via SetSentryMechanism before calling this (e.g. WinUI forwards the
275-
// platform's Handled value); only default to unhandled when nothing was declared.
274+
// Integrations always call `SetSentryMechanism` before calling this method (e.g. WinUI forwards the
275+
// platform's Handled value), so this fallback is defensive only. In practice, it never executes.
276276
if (!ex.Data.Contains(Mechanism.HandledKey))
277277
{
278278
ex.Data[Mechanism.HandledKey] = false;
@@ -295,12 +295,17 @@ public static SentryId CaptureException(this IHub hub, Exception ex, Action<Scop
295295
/// </summary>
296296
/// <param name="hub">The Sentry hub.</param>
297297
/// <param name="ex">The exception.</param>
298-
/// <param name="configureScope">The callback to configure the scope.</param>
299298
/// <param name="handled">Whether the exception was handled. Recorded on the exception, overriding any flag
300299
/// previously set on it, including one set via <see cref="SentryExceptionExtensions.SetSentryMechanism"/>.</param>
300+
/// <param name="configureScope">The callback to configure the scope.</param>
301301
/// <returns>The Id of the event</returns>
302-
public static SentryId CaptureException(this IHub hub, Exception ex, Action<Scope> configureScope, bool handled)
302+
public static SentryId CaptureException(this IHub hub, Exception ex, bool handled, Action<Scope> configureScope)
303303
{
304+
if (!hub.IsEnabled)
305+
{
306+
return SentryId.Empty;
307+
}
308+
304309
ex.Data[Mechanism.HandledKey] = handled;
305310
return hub.CaptureEvent(new SentryEvent(ex), configureScope);
306311
}

src/Sentry/SentrySdk.cs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -555,13 +555,13 @@ public static SentryId CaptureException(Exception exception, Action<Scope> confi
555555
/// This allows modifying a scope without affecting other events.
556556
/// </remarks>
557557
/// <param name="exception">The exception.</param>
558-
/// <param name="configureScope">The callback to configure the scope.</param>
559558
/// <param name="handled">Whether the exception was handled. Recorded on the exception, overriding any flag
560559
/// previously set on it, including one set via <see cref="SentryExceptionExtensions.SetSentryMechanism"/>.</param>
560+
/// <param name="configureScope">The callback to configure the scope.</param>
561561
/// <returns>The Id of the event.</returns>
562562
[DebuggerStepThrough]
563-
public static SentryId CaptureException(Exception exception, Action<Scope> configureScope, bool handled)
564-
=> CurrentHub.CaptureException(exception, configureScope, handled);
563+
public static SentryId CaptureException(Exception exception, bool handled, Action<Scope> configureScope)
564+
=> CurrentHub.CaptureException(exception, handled, configureScope);
565565

566566
/// <summary>
567567
/// Captures the message.

test/Sentry.Tests/ApiApprovalTests.Run.DotNet10_0.verified.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ namespace Sentry
148148
public static void AddBreadcrumb(this Sentry.IHub hub, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
149149
public static void AddBreadcrumb(this Sentry.IHub hub, Sentry.Infrastructure.ISystemClock? clock, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
150150
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope) { }
151-
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope, bool handled) { }
151+
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, bool handled, System.Action<Sentry.Scope> configureScope) { }
152152
public static Sentry.SentryId CaptureFeedback(this Sentry.IHub hub, Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
153153
public static Sentry.SentryId CaptureMessage(this Sentry.IHub hub, string message, System.Action<Sentry.Scope> configureScope, Sentry.SentryLevel level = 1) { }
154154
public static void LockScope(this Sentry.IHub hub) { }
@@ -981,7 +981,7 @@ namespace Sentry
981981
public static Sentry.SentryId CaptureException(System.Exception exception) { }
982982
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope) { }
983983
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled) { }
984-
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope, bool handled) { }
984+
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled, System.Action<Sentry.Scope> configureScope) { }
985985
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }
986986
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
987987
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, out Sentry.CaptureFeedbackResult result, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }

test/Sentry.Tests/ApiApprovalTests.Run.DotNet8_0.verified.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ namespace Sentry
148148
public static void AddBreadcrumb(this Sentry.IHub hub, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
149149
public static void AddBreadcrumb(this Sentry.IHub hub, Sentry.Infrastructure.ISystemClock? clock, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
150150
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope) { }
151-
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope, bool handled) { }
151+
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, bool handled, System.Action<Sentry.Scope> configureScope) { }
152152
public static Sentry.SentryId CaptureFeedback(this Sentry.IHub hub, Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
153153
public static Sentry.SentryId CaptureMessage(this Sentry.IHub hub, string message, System.Action<Sentry.Scope> configureScope, Sentry.SentryLevel level = 1) { }
154154
public static void LockScope(this Sentry.IHub hub) { }
@@ -981,7 +981,7 @@ namespace Sentry
981981
public static Sentry.SentryId CaptureException(System.Exception exception) { }
982982
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope) { }
983983
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled) { }
984-
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope, bool handled) { }
984+
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled, System.Action<Sentry.Scope> configureScope) { }
985985
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }
986986
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
987987
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, out Sentry.CaptureFeedbackResult result, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }

test/Sentry.Tests/ApiApprovalTests.Run.DotNet9_0.verified.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,7 @@ namespace Sentry
148148
public static void AddBreadcrumb(this Sentry.IHub hub, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
149149
public static void AddBreadcrumb(this Sentry.IHub hub, Sentry.Infrastructure.ISystemClock? clock, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
150150
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope) { }
151-
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope, bool handled) { }
151+
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, bool handled, System.Action<Sentry.Scope> configureScope) { }
152152
public static Sentry.SentryId CaptureFeedback(this Sentry.IHub hub, Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
153153
public static Sentry.SentryId CaptureMessage(this Sentry.IHub hub, string message, System.Action<Sentry.Scope> configureScope, Sentry.SentryLevel level = 1) { }
154154
public static void LockScope(this Sentry.IHub hub) { }
@@ -981,7 +981,7 @@ namespace Sentry
981981
public static Sentry.SentryId CaptureException(System.Exception exception) { }
982982
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope) { }
983983
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled) { }
984-
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope, bool handled) { }
984+
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled, System.Action<Sentry.Scope> configureScope) { }
985985
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }
986986
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
987987
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, out Sentry.CaptureFeedbackResult result, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }

test/Sentry.Tests/ApiApprovalTests.Run.Net4_8.verified.txt

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ namespace Sentry
136136
public static void AddBreadcrumb(this Sentry.IHub hub, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
137137
public static void AddBreadcrumb(this Sentry.IHub hub, Sentry.Infrastructure.ISystemClock? clock, string message, string? category = null, string? type = null, System.Collections.Generic.IDictionary<string, string>? data = null, Sentry.BreadcrumbLevel level = 0) { }
138138
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope) { }
139-
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, System.Action<Sentry.Scope> configureScope, bool handled) { }
139+
public static Sentry.SentryId CaptureException(this Sentry.IHub hub, System.Exception ex, bool handled, System.Action<Sentry.Scope> configureScope) { }
140140
public static Sentry.SentryId CaptureFeedback(this Sentry.IHub hub, Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
141141
public static Sentry.SentryId CaptureMessage(this Sentry.IHub hub, string message, System.Action<Sentry.Scope> configureScope, Sentry.SentryLevel level = 1) { }
142142
public static void LockScope(this Sentry.IHub hub) { }
@@ -962,7 +962,7 @@ namespace Sentry
962962
public static Sentry.SentryId CaptureException(System.Exception exception) { }
963963
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope) { }
964964
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled) { }
965-
public static Sentry.SentryId CaptureException(System.Exception exception, System.Action<Sentry.Scope> configureScope, bool handled) { }
965+
public static Sentry.SentryId CaptureException(System.Exception exception, bool handled, System.Action<Sentry.Scope> configureScope) { }
966966
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }
967967
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, System.Action<Sentry.Scope> configureScope, Sentry.SentryHint? hint = null) { }
968968
public static Sentry.SentryId CaptureFeedback(Sentry.SentryFeedback feedback, out Sentry.CaptureFeedbackResult result, Sentry.Scope? scope = null, Sentry.SentryHint? hint = null) { }

test/Sentry.Tests/HubExtensionsTests.cs

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ public void CaptureException_ScopeCallback_ExplicitHandled_RecordsFlagOnExceptio
9696
var ex = new Exception();
9797

9898
// Act
99-
_ = Sut.CaptureException(ex, _ => { }, handled);
99+
_ = Sut.CaptureException(ex, handled, _ => { });
100100

101101
// Assert
102102
Assert.Equal(handled, ex.Data[Mechanism.HandledKey]);
@@ -112,12 +112,28 @@ public void CaptureException_ScopeCallback_ExplicitHandled_OverridesFlagSetBySet
112112
ex.SetSentryMechanism("SomeMechanism", handled: true);
113113

114114
// Act
115-
_ = Sut.CaptureException(ex, _ => { }, handled: false);
115+
_ = Sut.CaptureException(ex, handled: false, _ => { });
116116

117117
// Assert
118118
Assert.Equal(false, ex.Data[Mechanism.HandledKey]);
119119
}
120120

121+
[Fact]
122+
public void CaptureException_ScopeCallback_ExplicitHandled_DisabledHub_DoesNotRecordFlagOnException()
123+
{
124+
// Arrange
125+
_ = Sut.IsEnabled.Returns(false);
126+
var ex = new Exception();
127+
128+
// Act
129+
var id = Sut.CaptureException(ex, handled: false, _ => { });
130+
131+
// Assert
132+
Assert.Equal(SentryId.Empty, id);
133+
Assert.False(ex.Data.Contains(Mechanism.HandledKey));
134+
_ = Sut.DidNotReceive().CaptureEvent(Arg.Any<SentryEvent>(), Arg.Any<Action<Scope>>());
135+
}
136+
121137
[Fact]
122138
public void CaptureExceptionInternal_NoPresetFlag_DefaultsToUnhandled()
123139
{

test/Sentry.Tests/SentrySdkTests.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -740,7 +740,7 @@ public void CaptureException_WithConfiguredScope_ExplicitHandled_SetsMechanismHa
740740
});
741741
});
742742

743-
SentrySdk.CaptureException(new Exception("test"), s => s.SetTag("scope-callback", "ran"), handled);
743+
SentrySdk.CaptureException(new Exception("test"), handled, s => s.SetTag("scope-callback", "ran"));
744744

745745
Assert.NotNull(captured);
746746
Assert.Equal(handled, Assert.Single(captured.SentryExceptions!).Mechanism!.Handled);

0 commit comments

Comments
 (0)