Update Sentry.NLog to use NLog 5

[mrfatmen]

trafficstars

Package

Sentry.Nlog

.NET Flavor

.NET

.NET Version

4.7.2

OS

Windows

SDK Version

3.17.1

Self-Hosted Sentry Version

No response

Steps to Reproduce

Use the NLog 5 nuget package.

Expected Result

No errors

Actual Result

System.TypeInitializationException HResult=0x80131534 Message=De type-initialisatiefunctie voor automate.logging.NLogLogger heeft een uitzondering veroorzaakt. Source=automate.logging StackTrace: at automate.logging.NLogLogger.Fatal(Exception x, Boolean LogExtern) in D:\sources\automate\automate.logging\NLogLogger.cs:line 214 at automate.App.ApplicationStartup(Object sender, StartupEventArgs e) in D:\sources\automate\automate\App.xaml.cs:line 1074 at System.Windows.Application.OnStartup(StartupEventArgs e) at System.Windows.Application.<.ctor>b__1_0(Object unused) at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs) at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)

This exception was originally thrown at this call stack: [External Code] automate.logging.NLogLogger.ConfigureLogger() in NLogLogger.cs automate.logging.NLogLogger.NLogLogger() in NLogLogger.cs

Inner Exception 1: FileLoadException: Kan bestand of assembly NLog, Version=4.0.0.0, Culture=neutral, PublicKeyToken=5120e14c03d0593c of een van de afhankelijkheden hiervan niet laden. De manifestdefinitie van de gevonden assembly komt niet overeen met de assembly-verwijzing. (Uitzondering van HRESULT: 0x80131040)

[SimonCropp]

@mrfatmen have you tried adding a binding redirect to v5 of nlog?

[mattjohnsonpint]

Looks like NLog 5 was recently released and has lots of breaking changes.

https://nlog-project.org/2022/05/16/nlog-5-0-finally-ready.html

First item says: "Strong Version Changed" - so I don't believe a binding redirect is possible. We will have to investigate further.

Thanks for bringing this to our attention.

[mattjohnsonpint]

Actually, I take that back. It looks like they just mean that they changed the major, not the public key token - and they advise binding redirects on that announcement post:

Can you try adding a binding redirect and see if that works for now? Thanks.

[SimonCropp]

@mattjohnsonpint given binding redirects are a pain, I would prefer we updated to nlog 5 and did a release

[mattjohnsonpint]

I agree, however that will be a breaking change, so we can do that in the next major release. I'll mark this as an enhancement to the 4.0.0 milestone.

@mrfatmen - For now, here is the binding redirect to add to your App.config file:

<?xml version="1.0" encoding="utf-8"?>
<configuration>
  <runtime>
    <assemblyBinding xmlns="urn:schemas-microsoft-com:asm.v1">

      <dependentAssembly>
        <assemblyIdentity name="NLog" publicKeyToken="5120e14c03d0593c" culture="neutral" />
        <bindingRedirect oldVersion="0.0.0.0-5.0.0.0" newVersion="5.0.0.0" />
      </dependentAssembly>

    </assemblyBinding>
  </runtime>
</configuration>

FYI, if you are using Visual Studio's Nuget GUI and add NLog 5 and then Sentry.NLog, it will add this redirect automatically.

[snakefoot]

The callstack doesn't seem to include any Sentry-classes:

at automate.logging.NLogLogger.Fatal(Exception x, Boolean LogExtern) in D:\sources\automate\automate.logging\NLogLogger.cs:line 214
at automate.App.ApplicationStartup(Object sender, StartupEventArgs e) in D:\sources\automate\automate\App.xaml.cs:line 1074
at System.Windows.Application.OnStartup(StartupEventArgs e)
at System.Windows.Application.<.ctor>b__1_0(Object unused)
at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs)
at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)

The pain of strong-version is only for .NET Framework-assemblies. All .NET Framework-assemblies must be compiled with the correct major-version-assemblies, which the application will be deployed with.

I guess if Sentry-Library removed the now unsupported .NET Framework 4.61-target (and just relied on .NET Standard2), then Sentry-Library would never be to blame. It would be the job of the application-owner to ensure all their relevant .NET Framework-project referenced the correct major-version-assembly at build-time.

[mrfatmen]

This seems to work. Thank you.

[mattjohnsonpint]

Glad to hear the binding redirect works. We'll keep this issue open though so that we remember to update NLog in a future release. Thanks.

[simonmolino]

hi @mattjohnsonpint - is there any update on this issue? or possible upgrade/fix?

We have a .net 4.8 framework dll that uses NLog v5 as well as referencing Sentry.NLog (using NLog v4).

We have added the binding redirect as suggested however still encounter the System.TypeInitializationException exception.

Our dll is being loaded by excel - as opposed to our own application.

If no upgrade/fix is scheduled, any idea on a possible workaround?

[mattjohnsonpint]

@simonmolino - I no longer work for Sentry, but I'm sure that @bruno-garcia can help you. Thanks.

[bitsandfoxes]

Hey @simonmolino, it still might be a little while for the update to land. I'm not sure I understand your setup: You're building a DLL that has Sentry as a dependency, and when that library gets loaded by Excel you encounter the TypeInitializationException? Could you provide us with a minimal repro?

[bitsandfoxes]

We upgraded to 5.0.5. This will be out with 4.0.0.