fix: Moved late option mutation into `Init` #2239

bitsandfoxes · 2025-07-10T13:42:07Z

This is a follow-up on #2227 as this was getting already kinda big.

Goal

This PR moves adding integrations that depend on certain flags into the Init call to be added "late" to keep the SDKs behavious consistent between self and manual initialization.

Context

There are two ways SentryUnityOptions gets created:

Self-Initializing: The SDK reads the .asset and creates new options from the ScriptableObject
Manual-Initialization: User code initializes the SDK

In both cases, changes to the options need to be made at the same time. This leaves either the constructor or right before initialization.

This means that handling those "late" integrations moved out of ScriptableUnityOptions - which is not getting called when initializing manually - into the initialization flow.

Note

One could point out the inconsistency how integrations get controlled and added, i.e. AnrIntegration, ScreenshotProcessor and ViewHierarchyProcessor. We could make changes to this but we'd require users to update from

options.AttachScreenshot = true;

to

options.AddEventProcessor(new ViewHierarchyEventProcessor(options));

which is not all that much better.

…tsentry/sentry-unity into fix/new-init-options-creation

bitsandfoxes · 2025-07-10T14:45:07Z

src/Sentry.Unity/ScriptableSentryUnityOptions.cs

-        if (options.AttachViewHierarchy)
-        {
-            options.AddEventProcessor(new ViewHierarchyEventProcessor(options));
-        }
-        if (options.AttachScreenshot)
-        {
-            options.AddEventProcessor(new ScreenshotEventProcessor(options));
-        }
-
-        if (!application.IsEditor && options.Il2CppLineNumberSupportEnabled && unityInfo is not null)
-        {
-            options.AddIl2CppExceptionProcessor(unityInfo);
-        }
-
-        HandlePlatformRestrictedOptions(options, unityInfo, application);


This got moved into SentryUnitySdk.Init to keep the SDKs behaviour consistent.

bitsandfoxes · 2025-07-10T14:45:43Z

src/Sentry.Unity/ScriptableSentryUnityOptions.cs

@@ -252,41 +246,6 @@ internal SentryUnityOptions ToSentryUnityOptions(bool isBuilding, ISentryUnityIn
        return options;
    }

-    internal void HandlePlatformRestrictedOptions(SentryUnityOptions options, ISentryUnityInfo? unityInfo, IApplication application)


Same as above. Even if users call Init the SDK should make an effort not to crash the game.

bitsandfoxes · 2025-07-10T14:46:11Z

src/Sentry.Unity/SentryUnityOptions.cs

-    internal SentryUnityOptions(bool isBuilding, IApplication application, ISentryUnityInfo? unityInfo = null) :
-        this(SentryMonoBehaviour.Instance, application, isBuilding, unityInfo)


Got rid of a redundant internal constructor.

bitsandfoxes · 2025-07-10T14:47:50Z

src/Sentry.Unity/SentryUnityOptionsExtensions.cs

-    internal static void AddIl2CppExceptionProcessor(this SentryUnityOptions options, ISentryUnityInfo unityInfo)
-    {
-        if (unityInfo.Il2CppMethods is not null)
-        {
-            options.AddExceptionProcessor(new UnityIl2CppEventExceptionProcessor(options, unityInfo));
-        }
-        else
-        {
-            options.DiagnosticLogger?.LogWarning("Failed to find required IL2CPP methods - Skipping line number support");
-        }
-    }


Moved to Init.

bitsandfoxes · 2025-07-10T14:48:11Z

src/Sentry.Unity/SentryUnitySdk.cs

+    internal static void HandleWindowsPlayer(SentryUnitySdk unitySdk, SentryUnityOptions options)
+    {
+        // On Windows-Standalone, we disable cache dir in case multiple app instances run over the same path.
+        // Note: we cannot use a named Mutex, because Unity doesn't support it. Instead, we create a file with `FileShare.None`.
+        // https://forum.unity.com/threads/unsupported-internal-call-for-il2cpp-mutex-createmutex_internal-named-mutexes-are-not-supported.387334/
+        if (ApplicationAdapter.Instance.Platform is not RuntimePlatform.WindowsPlayer ||
+            options.CacheDirectoryPath is null)
+        {
+            return;
+        }
+
+        try
+        {
+            unitySdk._lockFile = new FileStream(Path.Combine(options.CacheDirectoryPath, "sentry-unity.lock"), FileMode.OpenOrCreate,
+                FileAccess.ReadWrite, FileShare.None);
+        }
+        catch (Exception ex)
+        {
+            options.DiagnosticLogger?.LogWarning("An exception was thrown while trying to " +
+                                                 "acquire a lockfile on the config directory: .NET event cache will be disabled.", ex);
+            options.CacheDirectoryPath = null;
+            options.AutoSessionTracking = false;
+        }
+    }


To keep the Init more readable.

bitsandfoxes · 2025-07-10T14:48:32Z

src/Sentry.Unity/SentryUnitySdk.cs

+        if (!ApplicationAdapter.Instance.IsEditor &&
+            (SentryPlatformServices.UnityInfo?.IL2CPP ?? false) &&
+            options.Il2CppLineNumberSupportEnabled)
+        {
+            if (SentryPlatformServices.UnityInfo.Il2CppMethods is not null)
+            {
+                options.AddExceptionProcessor(new UnityIl2CppEventExceptionProcessor(options));
+            }
+            else
+            {
+                options.DiagnosticLogger?.LogWarning("Failed to find required IL2CPP methods - Skipping line number support");
+            }


The IL2CPP Exception Processor is critical and "dangerous" as it calls into the IL2CPP backend and requires certain functions to be there. These functions change depending of Unity versions and are provided via IUnityInfo. As such, this does not get added by default but late during Init with extensive checks.

…try-unity into fix/initialization

bruno-garcia · 2025-07-10T19:31:26Z

This is a follow-up on #2227 as this was getting already kinda big.

"kinda" 😅

bruno-garcia · 2025-07-10T19:33:53Z

src/Sentry.Unity/Il2CppEventProcessor.cs

-            "Unity IL2CPP methods are not available.");
+
+        // We're throwing here but this should never happen. We're validating UnityInfo before adding the processor.
+        UnityInfo = SentryPlatformServices.UnityInfo


great! this was my only feedback in the other PR, validation that we're at the right state before going ahead

Also note that you copy a reference to use the same/consistent reference. This also means there's no path for a user to change these values after-the-fact.

But I understand that's the goal, and we then should follow this pattern when it makes sense. Instead of accessing the static mutable property in many places try to keep it to a minimum.

src/Sentry.Unity/SentryUnityOptions.cs

src/Sentry.Unity/SentryUnitySdk.cs

test/Sentry.Unity.Tests/ScriptableSentryUnityOptionsTests.cs

bruno-garcia · 2025-07-15T16:05:18Z

src/Sentry.Unity/SentryUnitySdk.cs

+
+        try
+        {
+            unitySdk._lockFile = new FileStream(Path.Combine(options.CacheDirectoryPath, "sentry-unity.lock"), FileMode.OpenOrCreate,


oh this looks odd. We're accessing a field from that instance in here? If that's internal name it LockFIle or something (Pascal Case)

bitsandfoxes added 30 commits July 3, 2025 17:39

initial working implementation

14427b6

fixed using in smoketester

bb4dbc0

we actually need both. idk if that's great. doesn't look great

0f97f0b

bumped .NET to have the comment

a76e957

Updated CHANGELOG.md

e494279

bumped to cleaned up version of .NET

df211a3

added services logging to init

19c4e78

added summary to platformservices

00e788a

fixed namespaces

bfcd0f1

cleaned up bugfarmbuttons

4448334

namespace clean followup

232833c

fixed qualification

bcbb551

moved scenetracing integration into the SDK

deb6aa7

updated CI to 2020

a66a5a6

env bump

857e492

removed now redundant test

eefad70

bumped uniy-of-bugs to 2020

16c9fcf

updated the package

6542706

removed now redundant asmdef

b48e60d

Updated CHANGELOG.md

76e05b2

missing option field?

3fa372c

merged unity 2019 drop

1f5e5cc

finished fixing the startup tracing

83deeec

merged main

011ba1d

bump android API level to 21 for 2020

2a46b5f

Merge branch 'fix/android-2020-ci' into fix/initialization

7ac68be

updated test to ignore harmless warning

43f5ae9

move startup tracing into the SDK

ad0ae02

.

bb6f80a

fix naming

c53ccb5

bitsandfoxes and others added 10 commits July 10, 2025 12:03

rename setup platform services

a4cbc59

public close cutback

32876ac

moved late integrations and sumsuch into init

0b8a02f

Updated CHANGELOG.md

7d0ffca

review

c6cbb0c

Merge branch 'fix/initialization' into fix/new-init-options-creation

2ed2686

Format code

3a2328b

Format code

4379954

Updated CHANGELOG.md

26567b3

Merge branch 'fix/new-init-options-creation' of https://github.com/ge…

24c4d94

…tsentry/sentry-unity into fix/new-init-options-creation

bitsandfoxes commented Jul 10, 2025

View reviewed changes

bitsandfoxes added 4 commits July 10, 2025 16:50

Merge branch 'main' into chore/cleanup-init

2e4b3db

Merge branch 'chore/cleanup-init' into fix/initialization

99bbb4d

Merge branch 'fix/initialization' of https://github.com/getsentry/sen…

d67b2b9

…try-unity into fix/initialization

Merge branch 'fix/initialization' into fix/new-init-options-creation

70222d3

bitsandfoxes requested review from bruno-garcia, vaind and Flash0ver July 10, 2025 14:52

bruno-garcia reviewed Jul 10, 2025

View reviewed changes

test/Sentry.Unity.Tests/ScriptableSentryUnityOptionsTests.cs Show resolved Hide resolved

test/Sentry.Unity.Tests/ScriptableSentryUnityOptionsTests.cs Show resolved Hide resolved

Base automatically changed from fix/initialization to main July 15, 2025 06:26

bitsandfoxes added 6 commits July 15, 2025 11:49

merged main

502873b

finished up on merge conflicts and ctor cleanup

e518e8e

final touches

4d2d621

missed a test

a618ce8

Updated CHANGELOG.md

758ba57

cleanup

1dd06bb

bitsandfoxes requested a review from bruno-garcia July 15, 2025 15:41

bruno-garcia approved these changes Jul 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix: Moved late option mutation into `Init` #2239

fix: Moved late option mutation into `Init` #2239

Uh oh!

bitsandfoxes commented Jul 10, 2025 •

edited

Loading

Uh oh!

bitsandfoxes Jul 10, 2025

Uh oh!

bitsandfoxes Jul 10, 2025

Uh oh!

bitsandfoxes Jul 10, 2025

Uh oh!

bitsandfoxes Jul 10, 2025

Uh oh!

bitsandfoxes Jul 10, 2025

Uh oh!

bitsandfoxes Jul 10, 2025

Uh oh!

bruno-garcia commented Jul 10, 2025

Uh oh!

bruno-garcia Jul 10, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bruno-garcia Jul 15, 2025

Uh oh!

Uh oh!

		internal SentryUnityOptions(bool isBuilding, IApplication application, ISentryUnityInfo? unityInfo = null) :
		this(SentryMonoBehaviour.Instance, application, isBuilding, unityInfo)

Uh oh!

fix: Moved late option mutation into Init #2239

Are you sure you want to change the base?

fix: Moved late option mutation into Init #2239

Uh oh!

Conversation

bitsandfoxes commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Goal

Context

Note

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bruno-garcia commented Jul 10, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fix: Moved late option mutation into `Init` #2239

fix: Moved late option mutation into `Init` #2239

bitsandfoxes commented Jul 10, 2025 •

edited

Loading