Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sentry debug logging crashes because of argument confusion in LogError() methods #1090

Closed
4 of 17 tasks
Zero3 opened this issue Jun 28, 2021 · 0 comments · Fixed by #1092
Closed
4 of 17 tasks

Sentry debug logging crashes because of argument confusion in LogError() methods #1090

Zero3 opened this issue Jun 28, 2021 · 0 comments · Fixed by #1092
Labels
Bug Something isn't working

Comments

@Zero3
Copy link

Zero3 commented Jun 28, 2021

Please mark the type framework used:

  • ASP.NET MVC
  • ASP.NET Web API (OWIN)
  • ASP.NET Core
  • WPF
  • WinForms
  • Xamarin
  • Other:

Please mark the type of the runtime used:

  • .NET Framework
  • Mono
  • .NET Core
  • Version: 4.8

Please mark the NuGet packages used:

  • Sentry
  • Sentry.AspNet (manually added by me, as it is missing in the template here in GitHub)
  • Sentry.Serilog
  • Sentry.NLog
  • Sentry.Log4Net
  • Sentry.Extensions.Logging
  • Sentry.AspNetCore
  • Version: 3.6.0

The following LogError() call is broken, and causes the Sentry SDK to crash when triggered:

_options.DiagnosticLogger?.LogError(
"Error while processing event {1}: {0}. #{2} in queue.",
exception,
envelope.TryGetEventId(),
_queue.Count
);

The problem is that the call resolves to the wrong overload of LogError() because of argument confusion in the overloads:

public static void LogError<TArg, TArg2>(
this IDiagnosticLogger logger,
string message,
Exception exception,
TArg arg,
TArg2 arg2)
=> logger.LogIfEnabled(SentryLevel.Error, message, exception, arg, arg2);

This overload only expects 2 formatting arguments, resulting in a System.FormatException further down the line because the message contains 3 placeholders.

I noticed this related issue that points out the problem with the current design of the overloads: #699

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working
Projects
None yet
3 participants