diff --git a/.gitignore b/.gitignore index c1a7a4b1e..8ba14ec97 100644 --- a/.gitignore +++ b/.gitignore @@ -95,3 +95,6 @@ test/Sentry.Unity.Tests/other/** samples/unity-of-bugs/*IL2CPPCache/ samples/unity-of-bugs/*IL2CPPStats/ *Player.link.log + +# Gradle caches generated by the VS Code Gradle extension scanning test fixtures +.gradle/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 39fb12fd0..175d82380 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ### Features +- The SDK now provides line number support for managed exceptions for Unity 6.5 and newer ([#2805](https://github.com/getsentry/sentry-unity/pull/2805)) - Added experimental auto game-metrics. When enabled, the SDK periodically collects common performance metrics and sends them to Sentry via the metrics API. ([#2777](https://github.com/getsentry/sentry-unity/pull/2777)) ### Dependencies diff --git a/src/Sentry.Unity.Editor/Il2CppBuildPreProcess.cs b/src/Sentry.Unity.Editor/Il2CppBuildPreProcess.cs index 5c41aa1cd..133d1dbcd 100644 --- a/src/Sentry.Unity.Editor/Il2CppBuildPreProcess.cs +++ b/src/Sentry.Unity.Editor/Il2CppBuildPreProcess.cs @@ -1,5 +1,6 @@ using System; using Sentry.Extensibility; +using Sentry.Unity.Integrations; using UnityEditor; using UnityEditor.Build; using UnityEditor.Build.Reporting; @@ -9,6 +10,7 @@ namespace Sentry.Unity.Editor; internal class Il2CppBuildPreProcess : IPreprocessBuildWithReport { internal const string SourceMappingArgument = "--emit-source-mapping"; + internal const string LinkSymbolsArgument = "--link-symbols"; private static IDiagnosticLogger? Logger; public int callbackOrder => 0; @@ -33,6 +35,41 @@ public void OnPreprocessBuild(BuildReport report) SetAdditionalIl2CppArguments(options, PlayerSettings.GetAdditionalIl2CppArgs, PlayerSettings.SetAdditionalIl2CppArgs); + + SetAdditionalUnityLinkerArguments(options, + () => UnityLinkerDiagnosticSwitch.GetValue(Logger), + arguments => UnityLinkerDiagnosticSwitch.SetValue(arguments, Logger)); + } + + // The 'VMUnityLinkerAdditionalArgs' diagnostic switch only exists starting with Unity 6.5. + internal static void SetAdditionalUnityLinkerArguments(SentryUnityOptions options, Func getArguments, Action setArguments, IApplication? application = null) + { + if (!SentryUnityVersion.IsNewerOrEqualThan("6000.5", application)) + { + Logger?.LogDebug("Unity 6.5 or newer required to set additional UnityLinker arguments. Skipping."); + return; + } + + var arguments = getArguments.Invoke(); + + if (options.Il2CppLineNumberSupportEnabled) + { + if (arguments?.Contains(LinkSymbolsArgument) == true) + { + Logger?.LogDebug("Additional UnityLinker argument '{0}' already present.", LinkSymbolsArgument); + return; + } + + Logger?.LogDebug("IL2CPP line number support enabled - Adding additional UnityLinker argument."); + setArguments.Invoke(string.IsNullOrWhiteSpace(arguments) + ? LinkSymbolsArgument + : $"{arguments} {LinkSymbolsArgument}"); + } + else if (arguments?.Contains(LinkSymbolsArgument) == true) + { + Logger?.LogDebug("IL2CPP line number support disabled - Removing additional UnityLinker argument."); + setArguments.Invoke(arguments.Replace(LinkSymbolsArgument, "").Trim()); + } } internal static void SetAdditionalIl2CppArguments(SentryUnityOptions options, Func getArguments, Action setArguments) diff --git a/src/Sentry.Unity.Editor/UnityLinkerDiagnosticSwitch.cs b/src/Sentry.Unity.Editor/UnityLinkerDiagnosticSwitch.cs new file mode 100644 index 000000000..f766ea329 --- /dev/null +++ b/src/Sentry.Unity.Editor/UnityLinkerDiagnosticSwitch.cs @@ -0,0 +1,95 @@ +using System; +using System.Reflection; +using Sentry.Extensibility; +using UnityEngine; + +namespace Sentry.Unity.Editor; + +/// +/// Provides access to Unity's 'VMUnityLinkerAdditionalArgs' diagnostic switch. Unity feeds its value into the +/// UnityLinker's additional arguments and, unlike the 'UNITYLINKER_ADDITIONAL_ARGS' environment variable, it is part +/// of the Bee build graph's inputs. That means changing it invalidates the cached graph the same way the additional +/// IL2CPP arguments do - without it the linker keeps running with whatever arguments the cached graph was built with. +/// The switch is internal to Unity, so we have to go through reflection to get to it. +/// +internal static class UnityLinkerDiagnosticSwitch +{ + internal const string SwitchName = "VMUnityLinkerAdditionalArgs"; + + public static string? GetValue(IDiagnosticLogger? logger = null) + { + var diagnosticSwitch = GetSwitch(logger); + if (diagnosticSwitch is null) + { + return null; + } + + try + { + return diagnosticSwitch.GetType().GetProperty("value")?.GetValue(diagnosticSwitch) as string; + } + catch (Exception e) + { + logger?.LogWarning("Failed to read the '{0}' diagnostic switch. Reason: {1}", SwitchName, e.Message); + return null; + } + } + + public static bool SetValue(string value, IDiagnosticLogger? logger = null) + { + var diagnosticSwitch = GetSwitch(logger); + if (diagnosticSwitch is null) + { + return false; + } + + try + { + var property = diagnosticSwitch.GetType().GetProperty("value"); + if (property is null) + { + logger?.LogWarning("Failed to resolve the value of the '{0}' diagnostic switch.", SwitchName); + return false; + } + + property.SetValue(diagnosticSwitch, value); + + // Reading the value straight back so we can tell a failed write apart from one that Unity does not pick up. + logger?.LogDebug("Set '{0}' to '{1}'. It now reads back as '{2}'.", + SwitchName, value, property.GetValue(diagnosticSwitch)); + + return true; + } + catch (Exception e) + { + logger?.LogWarning("Failed to set the '{0}' diagnostic switch. Reason: {1}", SwitchName, e.Message); + return false; + } + } + + private static object? GetSwitch(IDiagnosticLogger? logger) + { + try + { + var method = typeof(Debug).GetMethod("GetDiagnosticSwitch", BindingFlags.Static | BindingFlags.NonPublic); + if (method is null) + { + logger?.LogWarning("Failed to resolve 'Debug.GetDiagnosticSwitch'."); + return null; + } + + var diagnosticSwitch = method.Invoke(null, new object[] { SwitchName }); + if (diagnosticSwitch is null) + { + logger?.LogWarning("The diagnostic switch '{0}' does not exist.", SwitchName); + } + + return diagnosticSwitch; + } + catch (Exception e) + { + logger?.LogWarning("Failed to access the '{0}' diagnostic switch. Reason: {1}", SwitchName, e.Message); + return null; + } + } +} diff --git a/test/IntegrationTest/Integration.Tests.ps1 b/test/IntegrationTest/Integration.Tests.ps1 index 107993098..9dec4f8eb 100644 --- a/test/IntegrationTest/Integration.Tests.ps1 +++ b/test/IntegrationTest/Integration.Tests.ps1 @@ -348,6 +348,23 @@ Describe "Unity $($env:SENTRY_TEST_PLATFORM) Integration Tests" { $exception.stacktrace | Should -Not -BeNullOrEmpty } + It "Resolves the throw frame to its source line" { + if ($script:Platform -in "WebGL") { + Set-ItResult -Skipped -Because "Source-line assertions are unsupported on $script:Platform" + return + } + + $frame = $runEvent.exception.values[0].stacktrace.frames | + Where-Object { $_.module -eq "IntegrationTester" -and $_.function -eq "ThrowException" } | + Select-Object -First 1 + + $frame | Should -Not -BeNullOrEmpty + $frame.absPath | Should -Match "[\\/]Assets[\\/]Scripts[\\/]IntegrationTester\.cs$" + # Which line exactly gets reported differs between Unity versions, so we only assert that we resolved one. + $frame.lineNo | Should -BeGreaterThan 0 + $frame.symbolicatorStatus | Should -Be "symbolicated" + } + It "Has error level" { ($runEvent.tags | Where-Object { $_.key -eq "level" }).value | Should -Be "error" } diff --git a/test/Sentry.Unity.Editor.Tests/Il2CppBuildPreProcess.cs b/test/Sentry.Unity.Editor.Tests/Il2CppBuildPreProcess.cs index 78bcd81d1..3ca686001 100644 --- a/test/Sentry.Unity.Editor.Tests/Il2CppBuildPreProcess.cs +++ b/test/Sentry.Unity.Editor.Tests/Il2CppBuildPreProcess.cs @@ -1,10 +1,13 @@ using System; using NUnit.Framework; +using Sentry.Unity.Tests.Stubs; namespace Sentry.Unity.Editor.Tests; public class Il2CppBuildPreProcessTests { + private static readonly TestApplication SupportedUnity = new(unityVersion: "6000.5.0f1"); + private string arguments = null!; private string resultingArguments = null!; @@ -82,4 +85,72 @@ public void SetAdditionalArguments_Il2CppDisabledAndArgumentAlreadyAdded_Removes Assert.That(resultingArguments, Does.Contain(expectedArgument)); Assert.That(resultingArguments, Does.Not.Contain(Il2CppBuildPreProcess.SourceMappingArgument)); } + + [Test] + public void SetAdditionalUnityLinkerArguments_Il2CppEnabled_AddsArgument() + { + var options = new SentryUnityOptions { Il2CppLineNumberSupportEnabled = true }; + + Il2CppBuildPreProcess.SetAdditionalUnityLinkerArguments(options, () => null, s => resultingArguments = s, SupportedUnity); + + Assert.That(resultingArguments, Is.EqualTo(Il2CppBuildPreProcess.LinkSymbolsArgument)); + } + + [Test] + public void SetAdditionalUnityLinkerArguments_Il2CppDisabled_DoesNotAddArgument() + { + var options = new SentryUnityOptions { Il2CppLineNumberSupportEnabled = false }; + + Il2CppBuildPreProcess.SetAdditionalUnityLinkerArguments(options, () => null, s => resultingArguments = s, SupportedUnity); + + Assert.That(resultingArguments, Does.Not.Contain(Il2CppBuildPreProcess.LinkSymbolsArgument)); + } + + [Test] + public void SetAdditionalUnityLinkerArguments_Il2CppEnabled_ExistingArgumentsDoNotGetOverwritten() + { + var options = new SentryUnityOptions { Il2CppLineNumberSupportEnabled = true }; + var expectedArgument = "--MyArgument"; + + Il2CppBuildPreProcess.SetAdditionalUnityLinkerArguments(options, () => expectedArgument, s => resultingArguments = s, SupportedUnity); + + Assert.That(resultingArguments, Is.EqualTo($"{expectedArgument} {Il2CppBuildPreProcess.LinkSymbolsArgument}")); + } + + [Test] + public void SetAdditionalUnityLinkerArguments_Il2CppDisabledAndArgumentAlreadyAdded_RemovesArgument() + { + var options = new SentryUnityOptions { Il2CppLineNumberSupportEnabled = false }; + var expectedArgument = "--MyArgument"; + arguments = $"{expectedArgument} {Il2CppBuildPreProcess.LinkSymbolsArgument}"; + + Il2CppBuildPreProcess.SetAdditionalUnityLinkerArguments(options, () => arguments, s => resultingArguments = s, SupportedUnity); + + Assert.That(resultingArguments, Is.EqualTo(expectedArgument)); + } + + [Test] + public void SetAdditionalUnityLinkerArguments_ArgumentAlreadyAdded_AddsArgumentOnlyOnce() + { + var options = new SentryUnityOptions { Il2CppLineNumberSupportEnabled = true }; + arguments = $"--MyArgument {Il2CppBuildPreProcess.LinkSymbolsArgument}"; + + Il2CppBuildPreProcess.SetAdditionalUnityLinkerArguments(options, () => arguments, s => resultingArguments = s, SupportedUnity); + + Assert.That(resultingArguments, Is.Empty); + } + + [Test] + [TestCase("2021.3.0f1")] + [TestCase("6000.0.0f1")] + [TestCase("6000.4.9f1")] + public void SetAdditionalUnityLinkerArguments_UnsupportedUnityVersion_DoesNotSetArguments(string unityVersion) + { + var options = new SentryUnityOptions { Il2CppLineNumberSupportEnabled = true }; + var application = new TestApplication(unityVersion: unityVersion); + + Il2CppBuildPreProcess.SetAdditionalUnityLinkerArguments(options, () => null, s => resultingArguments = s, application); + + Assert.That(resultingArguments, Is.Empty); + } }