Skip to content

Commit ec8cc5b

Browse files
authored
fix: options.DisableLoggingIntegration() is no longer no-op (#2413)
1 parent 1e692f9 commit ec8cc5b

File tree

9 files changed

+194
-44
lines changed

9 files changed

+194
-44
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
### Fixes
1212

13+
- Fixed the extension methods on the options to disable unhandled exception capture and the logging integration ([#2413](https://github.com/getsentry/sentry-unity/pull/2413))
1314
- Fixed the breadcrumb format for lifecycle events ([#2407](https://github.com/getsentry/sentry-unity/pull/2407))
1415
- When targeting iOS, the `WatchdogTerminationIntegration` now defaults to `false` as to not report false positives. Users can control this through the option `IosWatchdogTerminationIntegrationEnabled` ([#2403](https://github.com/getsentry/sentry-unity/pull/2403))
1516
- The SDK now correctly sets the currently active scene's name on the event ([#2400](https://github.com/getsentry/sentry-unity/pull/2400))

src/Sentry.Unity/Integrations/UnityApplicationLoggingIntegration.cs

Lines changed: 3 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,11 @@ namespace Sentry.Unity.Integrations;
88

99
/// <summary>
1010
/// Hooks into Unity's `Application.LogMessageReceived` to capture breadcrumbs for Debug log methods
11-
/// and optionally capture LogError events. Does not handle `Debug.LogException` since it lacks the
12-
/// actual exception object needed for IL2CPP processing, except on WebGL where it's treated as a log message.
11+
/// and optionally capture LogError events. Does not handle `Debug.LogException` - exception handling
12+
/// is done by UnityLogHandlerIntegration (non-WebGL) or UnityWebGLExceptionHandler (WebGL).
1313
/// </summary>
1414
internal class UnityApplicationLoggingIntegration : ISdkIntegration
1515
{
16-
private readonly bool _captureExceptions;
1716
private readonly IApplication _application;
1817
private readonly ISystemClock _clock;
1918

@@ -24,9 +23,8 @@ internal class UnityApplicationLoggingIntegration : ISdkIntegration
2423
private IHub _hub = null!; // Set in Register
2524
private SentryUnityOptions _options = null!; // Set in Register
2625

27-
internal UnityApplicationLoggingIntegration(bool captureExceptions = false, IApplication? application = null, ISystemClock? clock = null)
26+
internal UnityApplicationLoggingIntegration(IApplication? application = null, ISystemClock? clock = null)
2827
{
29-
_captureExceptions = captureExceptions;
3028
_application = application ?? ApplicationAdapter.Instance;
3129
_clock = clock ?? SystemClock.Clock;
3230
}
@@ -59,7 +57,6 @@ internal void OnLogMessageReceived(string message, string stacktrace, LogType lo
5957
return;
6058
}
6159

62-
ProcessException(message, stacktrace, logType);
6360
ProcessError(message, stacktrace, logType);
6461
ProcessBreadcrumbs(message, logType);
6562
ProcessStructuredLog(message, logType);
@@ -82,19 +79,6 @@ private bool IsGettingDebounced(LogType logType)
8279
};
8380
}
8481

85-
private void ProcessException(string message, string stacktrace, LogType logType)
86-
{
87-
// LogType.Exception is getting handled by the `UnityLogHandlerIntegration`
88-
// UNLESS we're configured to process them - i.e. on WebGL
89-
if (logType is LogType.Exception && _captureExceptions)
90-
{
91-
_options.LogDebug("Exception capture has been enabled. Capturing exception through '{0}'.", nameof(UnityApplicationLoggingIntegration));
92-
93-
var evt = UnityLogEventFactory.CreateExceptionEvent(message, stacktrace, false, _options);
94-
_hub.CaptureEvent(evt);
95-
}
96-
}
97-
9882
private void ProcessError(string message, string stacktrace, LogType logType)
9983
{
10084
if (logType is not LogType.Error || !_options.CaptureLogErrorEvents)
Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
using System;
2+
using Sentry.Extensibility;
3+
using Sentry.Integrations;
4+
using UnityEngine;
5+
6+
namespace Sentry.Unity.Integrations;
7+
8+
/// <summary>
9+
/// WebGL-specific exception handler that captures exceptions through Application.LogMessageReceived.
10+
/// Required because UnityLogHandlerIntegration (Debug.unityLogger.logHandler) doesn't work on WebGL.
11+
/// </summary>
12+
internal sealed class UnityWebGLExceptionHandler : ISdkIntegration
13+
{
14+
private readonly IApplication _application;
15+
private IHub _hub = null!;
16+
private SentryUnityOptions _options = null!;
17+
private ErrorTimeDebounce _errorTimeDebounce = null!;
18+
19+
internal UnityWebGLExceptionHandler(IApplication? application = null)
20+
{
21+
_application = application ?? ApplicationAdapter.Instance;
22+
}
23+
24+
public void Register(IHub hub, SentryOptions sentryOptions)
25+
{
26+
_hub = hub ?? throw new ArgumentException("Hub is null.");
27+
_options = sentryOptions as SentryUnityOptions
28+
?? throw new ArgumentException("Options is not of type 'SentryUnityOptions'.");
29+
30+
_errorTimeDebounce = new ErrorTimeDebounce(_options.DebounceTimeError);
31+
_application.LogMessageReceived += OnLogMessageReceived;
32+
_application.Quitting += OnQuitting;
33+
}
34+
35+
internal void OnLogMessageReceived(string message, string stacktrace, LogType logType)
36+
{
37+
if (!_hub.IsEnabled)
38+
{
39+
return;
40+
}
41+
42+
if (logType is not LogType.Exception)
43+
{
44+
return;
45+
}
46+
47+
// We're not capturing the SDK's own logs
48+
if (message.StartsWith(UnityLogger.LogTag))
49+
{
50+
return;
51+
}
52+
53+
if (_options.EnableLogDebouncing && !_errorTimeDebounce.Debounced())
54+
{
55+
_options.LogDebug("Exception is getting debounced.");
56+
return;
57+
}
58+
59+
_options.LogDebug("Capturing exception on WebGL through LogMessageReceived.");
60+
var evt = UnityLogEventFactory.CreateExceptionEvent(message, stacktrace, false, _options);
61+
_hub.CaptureEvent(evt);
62+
}
63+
64+
private void OnQuitting() => _application.LogMessageReceived -= OnLogMessageReceived;
65+
}

src/Sentry.Unity/SentryUnityOptions.cs

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -350,13 +350,21 @@ internal SentryUnityOptions(IApplication? application = null,
350350
AddEventProcessor(processor);
351351
AddTransactionProcessor(processor);
352352

353-
// UnityLogHandlerIntegration is not compatible with WebGL, so it's added conditionally
354-
if (application.Platform != RuntimePlatform.WebGLPlayer)
353+
// Exception handling differs by platform due to WebGL limitations
354+
if (application.Platform == RuntimePlatform.WebGLPlayer)
355355
{
356+
// WebGL: UnityLogHandler doesn't work, use LogMessageReceived for exceptions
357+
AddIntegration(new UnityWebGLExceptionHandler());
358+
}
359+
else
360+
{
361+
// Standard platforms: Use UnityLogHandler for exceptions
356362
AddIntegration(new UnityLogHandlerIntegration());
357-
AddIntegration(new UnityApplicationLoggingIntegration());
358363
}
359364

365+
// All platforms use ApplicationLogging for logs/warnings/errors/breadcrumbs
366+
AddIntegration(new UnityApplicationLoggingIntegration());
367+
360368
AddIntegration(new StartupTracingIntegration());
361369
AddIntegration(new AnrIntegration(behaviour));
362370
AddIntegration(new UnityScopeIntegration(application));

src/Sentry.Unity/SentryUnityOptionsExtensions.cs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,23 @@ internal static void SetupUnityLogging(this SentryUnityOptions options)
8080
}
8181

8282
/// <summary>
83-
/// Disables the capture of errors through <see cref="UnityLogHandlerIntegration"/>.
83+
/// Disables the capture of logs, warnings, errors, breadcrumbs, and structured logs through
84+
/// <see cref="UnityApplicationLoggingIntegration"/>.
8485
/// </summary>
8586
/// <param name="options">The SentryUnityOptions to remove the integration from.</param>
86-
public static void DisableUnityApplicationLoggingIntegration(this SentryUnityOptions options) =>
87+
public static void DisableUnityLoggingIntegration(this SentryUnityOptions options) =>
88+
options.RemoveIntegration<UnityApplicationLoggingIntegration>();
89+
90+
/// <summary>
91+
/// Disables the capture of unhandled exceptions. This removes both <see cref="UnityLogHandlerIntegration"/>
92+
/// (non-WebGL platforms) and <see cref="UnityWebGLExceptionHandler"/> (WebGL platform).
93+
/// </summary>
94+
/// <param name="options">The SentryUnityOptions to remove the integration from.</param>
95+
public static void DisableUnhandledExceptionCapture(this SentryUnityOptions options)
96+
{
8797
options.RemoveIntegration<UnityLogHandlerIntegration>();
98+
options.RemoveIntegration<UnityWebGLExceptionHandler>();
99+
}
88100

89101
/// <summary>
90102
/// Disables the application-not-responding detection.

src/Sentry.Unity/WebGL/SentryWebGL.cs

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@ public static void Configure(SentryUnityOptions options)
1717
{
1818
options.DiagnosticLogger?.LogDebug("Updating configuration for Unity WebGL.");
1919

20-
// Due to special exception handling on WebGL we capture these through `LogMessageReceived` too
21-
options.AddIntegration(new UnityApplicationLoggingIntegration(captureExceptions: true));
22-
2320
// Note: we need to use a custom background worker which actually doesn't work in the background
2421
// because Unity doesn't support async (multithreading) yet. This may change in the future so let's watch
2522
// https://docs.unity3d.com/2019.4/Documentation/ScriptReference/PlayerSettings.WebGL-threadsSupport.html

test/Sentry.Unity.Tests/SentryUnityOptionsExtensionsTests.cs

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
using System.Linq;
12
using NUnit.Framework;
23
using Sentry.Unity.Tests.Stubs;
34

@@ -129,4 +130,57 @@ public void SetupLogging_DiagnosticLoggerSet_LeavesOrRemovesDiagnosticLogger(boo
129130

130131
Assert.AreEqual(debug, options.DiagnosticLogger is not null);
131132
}
133+
134+
[Test]
135+
public void DisableUnityLoggingIntegration_RemovesUnityApplicationLoggingIntegration()
136+
{
137+
var options = _fixture.GetSut();
138+
139+
Assert.IsTrue(options.Integrations.Any(i => i is Integrations.UnityApplicationLoggingIntegration));
140+
141+
options.DisableUnityLoggingIntegration();
142+
143+
Assert.IsFalse(options.Integrations.Any(i => i is Integrations.UnityApplicationLoggingIntegration));
144+
}
145+
146+
[Test]
147+
public void DisableUnhandledExceptionCapture_RemovesUnityLogHandlerIntegration()
148+
{
149+
var options = _fixture.GetSut();
150+
151+
Assert.IsTrue(options.Integrations.Any(i => i is Integrations.UnityLogHandlerIntegration));
152+
153+
options.DisableUnhandledExceptionCapture();
154+
155+
Assert.IsFalse(options.Integrations.Any(i => i is Integrations.UnityLogHandlerIntegration));
156+
}
157+
158+
[Test]
159+
public void DisableUnhandledExceptionCapture_RemovesUnityWebGLExceptionHandler()
160+
{
161+
var application = new TestApplication(isEditor: false, platform: UnityEngine.RuntimePlatform.WebGLPlayer);
162+
var options = new SentryUnityOptions(application: application)
163+
{
164+
Enabled = true,
165+
Dsn = "http://test.com",
166+
CaptureInEditor = true,
167+
Debug = true,
168+
};
169+
170+
Assert.IsTrue(options.Integrations.Any(i => i is Integrations.UnityWebGLExceptionHandler));
171+
172+
options.DisableUnhandledExceptionCapture();
173+
174+
Assert.IsFalse(options.Integrations.Any(i => i is Integrations.UnityWebGLExceptionHandler));
175+
}
176+
177+
[Test]
178+
public void DisableUnhandledExceptionCapture_DoesNotRemoveUnityApplicationLoggingIntegration()
179+
{
180+
var options = _fixture.GetSut();
181+
182+
options.DisableUnhandledExceptionCapture();
183+
184+
Assert.IsTrue(options.Integrations.Any(i => i is Integrations.UnityApplicationLoggingIntegration));
185+
}
132186
}

test/Sentry.Unity.Tests/UnityApplicationLoggingIntegrationTests.cs

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,10 @@ private class Fixture
1313
public TestHub Hub { get; set; } = null!;
1414
public SentryUnityOptions SentryOptions { get; set; } = null!;
1515

16-
public bool CaptureExceptions { get; set; } = false;
17-
1816
public UnityApplicationLoggingIntegration GetSut()
1917
{
2018
var application = new TestApplication();
21-
var integration = new UnityApplicationLoggingIntegration(CaptureExceptions, application, clock: null);
19+
var integration = new UnityApplicationLoggingIntegration(application, clock: null);
2220
integration.Register(Hub, SentryOptions);
2321
return integration;
2422
}
@@ -164,18 +162,6 @@ public void OnLogMessageReceived_AddAsBreadcrumbDisabled_NotAddedAsBreadcrumb(Lo
164162
Assert.IsFalse(_fixture.Hub.ConfigureScopeCalls.Count > 0);
165163
}
166164

167-
[Test]
168-
public void OnLogMessageReceived_LogTypeException_CaptureExceptionsEnabled_EventCaptured()
169-
{
170-
_fixture.CaptureExceptions = true;
171-
var sut = _fixture.GetSut();
172-
var message = TestContext.CurrentContext.Test.Name;
173-
174-
sut.OnLogMessageReceived(message, "stacktrace", LogType.Exception);
175-
176-
Assert.AreEqual(1, _fixture.Hub.CapturedEvents.Count);
177-
}
178-
179165
[Test]
180166
[TestCase(LogType.Log)]
181167
[TestCase(LogType.Warning)]
@@ -305,7 +291,6 @@ public void OnLogMessageReceived_ExperimentalCaptureEnabled_CapturesStructuredLo
305291
{
306292
_fixture.SentryOptions.EnableLogs = true;
307293
_fixture.SentryOptions.CaptureStructuredLogsForLogType[LogType.Exception] = true;
308-
_fixture.CaptureExceptions = true;
309294
var sut = _fixture.GetSut();
310295
var message = TestContext.CurrentContext.Test.Name;
311296

@@ -323,7 +308,6 @@ public void OnLogMessageReceived_ExperimentalCaptureDisabled_DoesNotCaptureStruc
323308
{
324309
_fixture.SentryOptions.EnableLogs = true;
325310
_fixture.SentryOptions.CaptureStructuredLogsForLogType[LogType.Exception] = false;
326-
_fixture.CaptureExceptions = true;
327311
var sut = _fixture.GetSut();
328312
var message = TestContext.CurrentContext.Test.Name;
329313

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
using NUnit.Framework;
2+
using Sentry.Unity.Integrations;
3+
using Sentry.Unity.Tests.Stubs;
4+
using UnityEngine;
5+
6+
namespace Sentry.Unity.Tests;
7+
8+
public class UnityWebGLExceptionHandlerTests
9+
{
10+
private class Fixture
11+
{
12+
public TestHub Hub { get; set; } = null!;
13+
public SentryUnityOptions SentryOptions { get; set; } = null!;
14+
15+
public UnityWebGLExceptionHandler GetSut()
16+
{
17+
var application = new TestApplication();
18+
var integration = new UnityWebGLExceptionHandler(application);
19+
integration.Register(Hub, SentryOptions);
20+
return integration;
21+
}
22+
}
23+
24+
private Fixture _fixture = null!;
25+
26+
[SetUp]
27+
public void SetUp()
28+
{
29+
_fixture = new Fixture
30+
{
31+
Hub = new TestHub(),
32+
SentryOptions = new SentryUnityOptions()
33+
};
34+
}
35+
[Test]
36+
public void OnLogMessageReceived_LogTypeException_CaptureExceptionsEnabled_EventCaptured()
37+
{
38+
var sut = _fixture.GetSut();
39+
var message = TestContext.CurrentContext.Test.Name;
40+
41+
sut.OnLogMessageReceived(message, "stacktrace", LogType.Exception);
42+
43+
Assert.AreEqual(1, _fixture.Hub.CapturedEvents.Count);
44+
}
45+
}

0 commit comments

Comments
 (0)