diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1271dd85f..cd63869a1 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,7 @@
### Fixes
+- Hardened the `UnityLogger` to no longer throw when failing to format a log message. ([#2832](https://github.com/getsentry/sentry-unity/pull/2832))
- IL2CPP line numbers now work on Android x86/x86_64 builds. il2cpp fails to report the image UUID there, so the SDK falls back to looking the debug image up by name ([#2817](https://github.com/getsentry/sentry-unity/pull/2817))
### Dependencies
diff --git a/src/Sentry.Unity/UnityLogger.cs b/src/Sentry.Unity/UnityLogger.cs
index 4f7ef6b0a..fc9672b7f 100644
--- a/src/Sentry.Unity/UnityLogger.cs
+++ b/src/Sentry.Unity/UnityLogger.cs
@@ -30,7 +30,56 @@ public void Log(SentryLevel logLevel, string? message, Exception? exception = nu
return;
}
- _logger.Log(GetUnityLogType(logLevel), LogTag, $"({logLevel.ToString()}) {Format(message, args)} {exception}");
+ // A diagnostic logger must never take its caller down with it. Callers routinely log from
+ // inside a `catch` that already handled the failure.
+ string logMessage;
+ try
+ {
+ logMessage = $"({logLevel.ToString()}) {Format(message, args)} {Describe(exception)}";
+ }
+ catch (Exception e)
+ {
+ // Keep the raw message - a bad format string shouldn't erase what the caller wanted to say.
+ logMessage = $"({logLevel.ToString()}) {message} (formatting failed: {e.GetType().Name})";
+ }
+
+ try
+ {
+ _logger.Log(GetUnityLogType(logLevel), LogTag, logMessage);
+ }
+ catch
+ {
+ // Reporting a logging failure would have to go through the very thing that just failed.
+ }
+ }
+
+ ///
+ /// Renders an exception without trusting , which walks the
+ /// stack trace and can throw on platforms with limited stack trace support, i.e. Nintendo Switch.
+ ///
+ private static string Describe(Exception? exception)
+ {
+ if (exception is null)
+ {
+ return Empty;
+ }
+
+ try
+ {
+ return exception.ToString();
+ }
+ catch (Exception e)
+ {
+ try
+ {
+ return $"{exception.GetType().FullName}: {exception.Message}" +
+ $" (stack trace unavailable: {e.GetType().Name})";
+ }
+ catch
+ {
+ return "";
+ }
+ }
}
internal static LogType GetUnityLogType(SentryLevel logLevel)
diff --git a/test/Sentry.Unity.Tests/UnityLoggerTests.cs b/test/Sentry.Unity.Tests/UnityLoggerTests.cs
index daad31632..cbd763b07 100644
--- a/test/Sentry.Unity.Tests/UnityLoggerTests.cs
+++ b/test/Sentry.Unity.Tests/UnityLoggerTests.cs
@@ -1,3 +1,4 @@
+using System;
using NUnit.Framework;
using Sentry.Unity.Tests.SharedClasses;
using UnityEngine;
@@ -50,4 +51,52 @@ public void Log_SetsTag()
// The format is: "(logType, tag, message)"
StringAssert.AreEqualIgnoringCase(UnityLogger.LogTag, testLogger.Logs[0].Item2);
}
+
+ ///
+ /// Callers log from inside `catch` blocks that already handled the failure.
+ ///
+ [Test]
+ public void Log_ExceptionThrowsWhileBeingStringified_DoesNotPropagateAndStillLogs()
+ {
+ var testLogger = new UnityTestLogger();
+ var logger = new UnityLogger(new SentryOptions { DiagnosticLevel = SentryLevel.Debug }, testLogger);
+
+ Assert.DoesNotThrow(() =>
+ logger.Log(SentryLevel.Error, "Something failed", new ThrowingToStringException()));
+
+ Assert.AreEqual(1, testLogger.Logs.Count);
+ var message = testLogger.Logs[0].Item3;
+ StringAssert.Contains("Something failed", message);
+ StringAssert.Contains(nameof(ThrowingToStringException), message);
+ }
+
+ [Test]
+ public void Log_MessageAndArgumentsDoNotMatch_DoesNotPropagateAndKeepsTheRawMessage()
+ {
+ var testLogger = new UnityTestLogger();
+ var logger = new UnityLogger(new SentryOptions { DiagnosticLevel = SentryLevel.Debug }, testLogger);
+
+ // More placeholders than arguments - string.Format throws on this.
+ Assert.DoesNotThrow(() => logger.Log(SentryLevel.Debug, "{0} {1} {2}", null, "only-one"));
+
+ Assert.AreEqual(1, testLogger.Logs.Count);
+ // The unformatted message is still worth more than a bare "formatting failed".
+ StringAssert.Contains("{0} {1} {2}", testLogger.Logs[0].Item3);
+ }
+
+ private sealed class ThrowingToStringException : Exception
+ {
+ public ThrowingToStringException()
+ { }
+
+ public ThrowingToStringException(string message) : base(message)
+ { }
+
+ public ThrowingToStringException(string message, Exception innerException) : base(message, innerException)
+ { }
+
+ public override string ToString() => throw new InvalidOperationException("cannot stringify");
+
+ public override string StackTrace => throw new InvalidOperationException("no stack trace here");
+ }
}