Change signature order in DiagnosticLogger for exceptions (#2715)

CHANGELOG.md

@@ -17,18 +17,19 @@ without native/platform specific bindings and SDKs. See [this ticket for more de
 API Changes:
 
 - IHasMeasurements was removed. Use ISpanData instead. ([#2659](https://github.com/getsentry/sentry-dotnet/pull/2659))
-- If `null` has been supplied as DSN when initializing Sentry, and ArgumentNullException is now thrown ([#2655](https://github.com/getsentry/sentry-dotnet/pull/2655))
+- If `null` has been supplied as DSN when initializing Sentry, and ArgumentNullException is now thrown. ([#2655](https://github.com/getsentry/sentry-dotnet/pull/2655))
 - IHasBreadcrumbs was removed. Use IEventLike instead. ([#2670](https://github.com/getsentry/sentry-dotnet/pull/2670))
 - ISpanContext was removed. Use ITraceContext instead. ([#2668](https://github.com/getsentry/sentry-dotnet/pull/2668))
 - Removed IHasTransactionNameSource. Use ITransactionContext instead. ([#2654](https://github.com/getsentry/sentry-dotnet/pull/2654))
-- Adding `Distribution` to `IEventLike` ([#2660](https://github.com/getsentry/sentry-dotnet/pull/2660))
-- Upgraded to NLog version 5 ([#2697](https://github.com/getsentry/sentry-dotnet/pull/2697))
+- Adding `Distribution` to `IEventLike`. ([#2660](https://github.com/getsentry/sentry-dotnet/pull/2660))
+- Upgraded to NLog version 5. ([#2697](https://github.com/getsentry/sentry-dotnet/pull/2697))
 - Removed unused `StackFrame.InstructionOffset`. ([#2691](https://github.com/getsentry/sentry-dotnet/pull/2691))
 - Change `StackFrame`'s `ImageAddress`, `InstructionAddress` and `FunctionId` to `long?`. ([#2691](https://github.com/getsentry/sentry-dotnet/pull/2691))
-- Enable `CaptureFailedRequests` by default ([2688](https://github.com/getsentry/sentry-dotnet/pull/2688))
+- Enable `CaptureFailedRequests` by default. ([2688](https://github.com/getsentry/sentry-dotnet/pull/2688))
 - Additional constructors removed from `TransactionTracer`. ([#2694](https://github.com/getsentry/sentry-dotnet/pull/2694))
-- Removed the `Scope.Platform` property as it was never applied ([#2695](https://github.com/getsentry/sentry-dotnet/pull/2695))
+- Removed the `Scope.Platform` property as it was never applied. ([#2695](https://github.com/getsentry/sentry-dotnet/pull/2695))
 - Reordered parameters for ther TransactionContext and SpanContext constructors. If you're constructing instances of these classes, you will need to adjust the order in which you pass parameters to these. ([#2696](https://github.com/getsentry/sentry-dotnet/pull/2696))
+- The `DiagnosticLogger` signature for `LogError` and `LogFatal` changed to take the `exception` as the first parameter. That way it does no longer get mixed up with the TArgs. The `DiagnosticLogger` now also received an overload for `LogError` and `LogFatal` that accepts a message only. ([#2715](https://github.com/getsentry/sentry-dotnet/pull/2715))
 
 ## Unreleased
 

src/Sentry.AspNet/HttpContextExtensions.cs

@@ -26,7 +26,7 @@ public static class HttpContextExtensions
         }
         catch (Exception ex)
         {
-            options?.LogError("Invalid Sentry trace header '{0}'.", ex, value);
+            options?.LogError(ex, "Invalid Sentry trace header '{0}'.", value);
             return null;
         }
     }
@@ -50,7 +50,7 @@ public static class HttpContextExtensions
         }
         catch (Exception ex)
         {
-            options?.LogError("Invalid baggage header '{0}'.", ex, value);
+            options?.LogError(ex, "Invalid baggage header '{0}'.", value);
             return null;
         }
     }

src/Sentry.AspNetCore/Extensions/HttpContextExtensions.cs

@@ -64,7 +64,7 @@ internal static class HttpContextExtensions
         }
         catch (Exception ex)
         {
-            options?.LogError("Invalid Sentry trace header '{0}'.", ex, value);
+            options?.LogError(ex, "Invalid Sentry trace header '{0}'.", value);
             return null;
         }
     }
@@ -88,7 +88,7 @@ internal static class HttpContextExtensions
         }
         catch (Exception ex)
         {
-            options?.LogError("Invalid baggage header '{0}'.", ex, value);
+            options?.LogError(ex, "Invalid baggage header '{0}'.", value);
             return null;
         }
     }

src/Sentry.AspNetCore/ScopeExtensions.cs

@@ -64,7 +64,7 @@ public static void Populate(this Scope scope, HttpContext context, SentryAspNetC
         }
         catch (Exception e)
         {
-            options.LogError("Failed to extract body.", e);
+            options.LogError(e, "Failed to extract body.");
         }
 
         SetEnv(scope, context, options);

src/Sentry.AspNetCore/SentryTracingMiddleware.cs

@@ -90,7 +90,7 @@ internal class SentryTracingMiddleware
         }
         catch (Exception ex)
         {
-            _options.LogError("Failed to start transaction.", ex);
+            _options.LogError(ex, "Failed to start transaction.");
             return null;
         }
     }

src/Sentry.Azure.Functions.Worker/HttpRequestDataExtensions.cs

@@ -25,7 +25,7 @@ internal static class HttpRequestDataExtensions
         }
         catch (Exception ex)
         {
-            logger?.LogError("Invalid Sentry trace header '{0}'.", ex, traceHeaderValue);
+            logger?.LogError(ex, "Invalid Sentry trace header '{0}'.", traceHeaderValue);
             return null;
         }
     }
@@ -53,7 +53,7 @@ internal static class HttpRequestDataExtensions
         }
         catch (Exception ex)
         {
-            logger?.LogError("Invalid baggage header '{0}'.", ex, baggageValue);
+            logger?.LogError(ex, "Invalid baggage header '{0}'.", baggageValue);
             return null;
         }
     }

src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentryEFCoreListener.cs

@@ -88,7 +88,7 @@ public void OnNext(KeyValuePair<string, object?> value)
         }
         catch (Exception ex)
         {
-            _options.LogError("Failed to intercept EF Core event.", ex);
+            _options.LogError(ex, "Failed to intercept EF Core event.");
         }
     }
 }

src/Sentry.DiagnosticSource/Internal/DiagnosticSource/SentrySqlListener.cs

@@ -244,7 +244,7 @@ public void OnNext(KeyValuePair<string, object?> kvp)
         }
         catch (Exception ex)
         {
-            _options.LogError("Failed to intercept SQL event.", ex);
+            _options.LogError(ex, "Failed to intercept SQL event.");
         }
     }
 

src/Sentry.EntityFramework/SentryOptionsExtensions.cs

@@ -28,7 +28,7 @@ public static SentryOptions AddEntityFramework(this SentryOptions sentryOptions)
         catch (Exception e)
         {
             sentryOptions.DiagnosticLogger?
-                .LogError("Failed to configure EF breadcrumbs. Make sure to init Sentry before EF.", e);
+                .LogError(e, "Failed to configure EF breadcrumbs. Make sure to init Sentry before EF.");
         }
 
         dbIntegration = new DbInterceptionIntegration();

src/Sentry.Maui/Internal/MauiDeviceData.cs

@@ -112,7 +112,7 @@ public static void ApplyMauiDeviceData(this Device device, IDiagnosticLogger? lo
         catch (Exception ex)
         {
             // Log, but swallow the exception so we can continue sending events
-            logger?.LogError("Error getting MAUI device information.", ex);
+            logger?.LogError(ex, "Error getting MAUI device information.");
         }
     }
 }

src/Sentry.Maui/Internal/MauiEventsBinder.cs

@@ -143,7 +143,7 @@ public void BindReflectedEvents(BindableObject bindableObject, bool includeExpli
             catch (Exception ex)
             {
                 // Don't throw if we can't bind the event handler
-                _options.DiagnosticLogger?.LogError("Couldn't bind to {0}.{1}", ex, type.Name, eventInfo.Name);
+                _options.DiagnosticLogger?.LogError(ex, "Couldn't bind to {0}.{1}", type.Name, eventInfo.Name);
             }
         }
     }

src/Sentry.Maui/Internal/MauiOsData.cs

@@ -40,7 +40,7 @@ public static void ApplyMauiOsData(this OperatingSystem os, IDiagnosticLogger? l
         catch (Exception ex)
         {
             // Log, but swallow the exception so we can continue sending events
-            logger?.LogError("Error getting MAUI OS information.", ex);
+            logger?.LogError(ex, "Error getting MAUI OS information.");
         }
     }
 }

src/Sentry.NLog/SentryTarget.cs

@@ -303,7 +303,7 @@ protected override void Write(LogEventInfo logEvent)
         }
         catch (Exception exception)
         {
-            Options.DiagnosticLogger?.LogError("Failed to write log event", exception);
+            Options.DiagnosticLogger?.LogError(exception, "Failed to write log event");
             throw;
         }
         finally

src/Sentry.Profiling/SamplingTransactionProfilerFactory.cs

@@ -51,7 +51,7 @@ private SamplingTransactionProfilerFactory(SentryOptions options, SampleProfiler
             }
             catch (Exception e)
             {
-                _options.LogError("Failed to start a profiler session.", e);
+                _options.LogError(e, "Failed to start a profiler session.");
                 _inProgress = FALSE;
             }
         }

src/Sentry/Extensibility/DiagnosticLoggerExtensions.cs

@@ -211,68 +211,77 @@ public static class DiagnosticLoggerExtensions
     /// </summary>
     public static void LogError(
         this IDiagnosticLogger logger,
-        string message,
-        Exception? exception = null)
+        string message)
+        => logger.LogIfEnabled(SentryLevel.Error, null, message);
+
+    /// <summary>
+    /// Log an exception with an error message.
+    /// </summary>
+    public static void LogError(this IDiagnosticLogger logger,
+        Exception exception,
+        string message)
         => logger.LogIfEnabled(SentryLevel.Error, exception, message);
 
     /// <summary>
     /// Log a error message.
     /// </summary>
     internal static void LogError(
         this SentryOptions options,
-        string message,
-        Exception? exception = null)
+        string message)
+        => options.DiagnosticLogger?.LogIfEnabled(SentryLevel.Error, null, message);
+
+    /// <summary>
+    /// Log a error message.
+    /// </summary>
+    internal static void LogError(this SentryOptions options,
+        Exception exception,
+        string message)
         => options.DiagnosticLogger?.LogIfEnabled(SentryLevel.Error, exception, message);
 
     /// <summary>
     /// Log a error message.
     /// </summary>
-    public static void LogError<TArg>(
-        this IDiagnosticLogger logger,
-        string message,
+    public static void LogError<TArg>(this IDiagnosticLogger logger,
         Exception exception,
+        string message,
         TArg arg)
         => logger.LogIfEnabled(SentryLevel.Error, exception, message, arg);
 
     /// <summary>
     /// Log a error message.
     /// </summary>
-    internal static void LogError<TArg>(
-        this SentryOptions options,
-        string message,
+    internal static void LogError<TArg>(this SentryOptions options,
         Exception exception,
+        string message,
         TArg arg)
         => options.DiagnosticLogger?.LogIfEnabled(SentryLevel.Error, exception, message, arg);
 
     /// <summary>
     /// Log a error message.
     /// </summary>
-    public static void LogError<TArg, TArg2>(
-        this IDiagnosticLogger logger,
-        string message,
+    public static void LogError<TArg, TArg2>(this IDiagnosticLogger logger,
         Exception exception,
+        string message,
         TArg arg,
         TArg2 arg2)
         => logger.LogIfEnabled(SentryLevel.Error, exception, message, arg, arg2);
 
     /// <summary>
     /// Log a error message.
     /// </summary>
-    internal static void LogError<TArg, TArg2>(
-        this SentryOptions options,
-        string message,
+    internal static void LogError<TArg, TArg2>(this SentryOptions options,
         Exception exception,
+        string message,
         TArg arg,
         TArg2 arg2)
         => options.DiagnosticLogger?.LogIfEnabled(SentryLevel.Error, exception, message, arg, arg2);
 
     /// <summary>
     /// Log a error message.
     /// </summary>
-    public static void LogError<TArg, TArg2, TArg3, TArg4>(
-        this IDiagnosticLogger logger,
-        string message,
+    public static void LogError<TArg, TArg2, TArg3, TArg4>(this IDiagnosticLogger logger,
         Exception exception,
+        string message,
         TArg arg,
         TArg2 arg2,
         TArg3 arg3,
@@ -282,10 +291,9 @@ public static class DiagnosticLoggerExtensions
     /// <summary>
     /// Log a error message.
     /// </summary>
-    internal static void LogError<TArg, TArg2, TArg3, TArg4>(
-        this SentryOptions options,
-        string message,
+    internal static void LogError<TArg, TArg2, TArg3, TArg4>(this SentryOptions options,
         Exception exception,
+        string message,
         TArg arg,
         TArg2 arg2,
         TArg3 arg3,
@@ -321,17 +329,31 @@ public static class DiagnosticLoggerExtensions
     /// </summary>
     public static void LogFatal(
         this IDiagnosticLogger logger,
-        string message,
-        Exception? exception = null)
+        string message)
+        => logger.LogIfEnabled(SentryLevel.Fatal, null, message);
+
+    /// <summary>
+    /// Log an exception with a warning message.
+    /// </summary>
+    public static void LogFatal(this IDiagnosticLogger logger,
+        Exception exception,
+        string message)
         => logger.LogIfEnabled(SentryLevel.Fatal, exception, message);
 
     /// <summary>
     /// Log a warning message.
     /// </summary>
     internal static void LogFatal(
         this SentryOptions options,
-        string message,
-        Exception? exception = null)
+        string message)
+        => options.DiagnosticLogger?.LogIfEnabled(SentryLevel.Fatal, null, message);
+
+    /// <summary>
+    /// Log an exception with a warning message.
+    /// </summary>
+    internal static void LogFatal(this SentryOptions options,
+        Exception exception,
+        string message)
         => options.DiagnosticLogger?.LogIfEnabled(SentryLevel.Fatal, exception, message);
 
     internal static void LogIfEnabled(

src/Sentry/GlobalSessionManager.cs

@@ -85,7 +85,7 @@ internal class GlobalSessionManager : ISessionManager
         // and let the next installation id strategy kick in
         catch (Exception ex)
         {
-            _options.LogError("Failed to resolve persistent installation ID.", ex);
+            _options.LogError(ex, "Failed to resolve persistent installation ID.");
             return null;
         }
     }
@@ -113,7 +113,7 @@ internal class GlobalSessionManager : ISessionManager
         }
         catch (Exception ex)
         {
-            _options.LogError("Failed to resolve hardware installation ID.", ex);
+            _options.LogError(ex, "Failed to resolve hardware installation ID.");
             return null;
         }
     }
@@ -187,7 +187,7 @@ private void PersistSession(SessionUpdate update, DateTimeOffset? pauseTimestamp
         }
         catch (Exception ex)
         {
-            _options.LogError("Failed to persist session on the file system.", ex);
+            _options.LogError(ex, "Failed to persist session on the file system.");
         }
     }
 
@@ -212,7 +212,7 @@ private void DeletePersistedSession()
                 }
                 catch (Exception ex)
                 {
-                    _options.LogError("Failed to read the contents of persisted session file '{0}'.", ex, filePath);
+                    _options.LogError(ex, "Failed to read the contents of persisted session file '{0}'.", filePath);
                 }
             }
 
@@ -222,7 +222,7 @@ private void DeletePersistedSession()
         }
         catch (Exception ex)
         {
-            _options.LogError("Failed to delete persisted session from the file system: '{0}'", ex, filePath);
+            _options.LogError(ex, "Failed to delete persisted session from the file system: '{0}'", filePath);
         }
     }
 
@@ -256,7 +256,7 @@ private void DeletePersistedSession()
             }
             catch (Exception e)
             {
-                _options.LogError("Invoking CrashedLastRun failed.", e);
+                _options.LogError(e, "Invoking CrashedLastRun failed.");
             }
 
             // Create a session update to end the recovered session
@@ -285,7 +285,7 @@ private void DeletePersistedSession()
         }
         catch (Exception ex)
         {
-            _options.LogError("Failed to recover persisted session from the file system '{0}'.", ex, filePath);
+            _options.LogError(ex, "Failed to recover persisted session from the file system '{0}'.", filePath);
 
             return null;
         }

src/Sentry/Integrations/NetFxInstallationsIntegration.cs

@@ -17,7 +17,7 @@ public void Register(IHub hub, SentryOptions options)
         }
         catch (Exception ex)
         {
-            options.LogError("Failed to register NetFxInstallations.", ex);
+            options.LogError(ex, "Failed to register NetFxInstallations.");
         }
     }
 }