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

Write exceptions in InvariantCulture to log file #172

Closed
habakuk007 opened this issue Jan 22, 2013 · 19 comments
Closed

Write exceptions in InvariantCulture to log file #172

habakuk007 opened this issue Jan 22, 2013 · 19 comments

Comments

@habakuk007
Copy link

If you ever got a log file from Japan you got the problem.

Exceptions (and other strings that come from a resource) should be written to the log file with the CultureInfo.InvariantCulture.

@ghost
Copy link

ghost commented Jan 26, 2013

I can see want you mean, but what part of the exception do you need to be invariant culture? Isn't this a some the exception should contain?

@ghost
Copy link

ghost commented Jan 26, 2013

Seems to a duplicate #152

@ghost ghost closed this as completed Jan 26, 2013
@srollinet
Copy link

This is an old issue but it seems the fix done for #152 doesn't work.

The problem is when using a layout like ${exception:format=tostring} the exception is written in the CurrentUICulture. It would be great to have the option to set the CultureInfo to avoid this problem.

This behavior occurs even when setting useInvariantCulture="true"

Now the workaround is to switch the CurrentUICulture before each call to the logger and restore it after.

@304NotModified
Copy link
Member

Hi @srollinet,

I think #1528 is related? See also PR: #1556

@srollinet
Copy link

srollinet commented Jul 26, 2016

Hi @304NotModified ,

I'm not sure it is the same. What I expect is the possibility to have exactly the same exception message + stack trace regardless of the CurrentUICulture

Something like that:

[Fact]
public void ExceptionTest()
{
    var target = new MemoryTarget {Layout = @"${exception:format=tostring}"};
    SimpleConfigurator.ConfigureForTargetLogging(target);
    var logger = LogManager.GetCurrentClassLogger();

    try
    {
        throw new InvalidOperationException();
    }
    catch (Exception ex)
    {
        Thread.CurrentThread.CurrentUICulture = new CultureInfo("en-US");
        logger.Error(ex, "");

        Thread.CurrentThread.CurrentUICulture = new CultureInfo("de-DE");
        logger.Error(ex, "");

        Assert.Equal(target.Logs[0], target.Logs[1]);
    }
}

@304NotModified
Copy link
Member

@srollinet I added the unit tests to #1556 but it works for me locally (let see what appVeyor tells)

#1556 didn't changed the ExceptionLayoutRenderer, so it also works on master then.

Can you give an example of the visual differences you get? (please do a toString on target.Logs[0] and target.Logs[1] and post them here

@srollinet
Copy link

Tested with V4.3.6 (oops, I haven't seen that #1556 is not part of this release) so maybe the problem is simply here...

System.InvalidOperationException: Operation is not valid due to the current state of the object.
   at TestsSRO.ExceptionCultureTest.ExceptionTest() in C:\Users\sro\Documents\Visual Studio 2015\Projects\TestsSRO\TestsSRO\ExceptionCultureTest.cs:line 22
System.InvalidOperationException: Operation is not valid due to the current state of the object.
   bei TestsSRO.ExceptionCultureTest.ExceptionTest() in C:\Users\sro\Documents\Visual Studio 2015\Projects\TestsSRO\TestsSRO\ExceptionCultureTest.cs:Zeile 22.

@304NotModified
Copy link
Member

this is master (works):
image

But I think it works as I don't have any non-english languages installed on my machine. (or on AppVeyor)

@304NotModified
Copy link
Member

304NotModified commented Jul 26, 2016

@srollinet is it possible to test with a NuGet beta package? (will create one then)

@304NotModified
Copy link
Member

Dived into to this,

it seems not to be possible to pass the culture to the exception.toString or something like that. See http://stackoverflow.com/questions/209133/exception-messages-in-english

@srollinet
Copy link

srollinet commented Jul 26, 2016

@304NotModified I cloned the repository and the test fails as well. So yes it is probably because I have non-english languages installed.

image

I have seen the StackOverflow question and also asked a new one specific to NLog http://stackoverflow.com/questions/38585028/configure-nlog-to-write-exception-messages-in-english

As I say in my question, I wanted to use a better solution than switching manually the CurrentUICulture. But if it is not possible I will probably wrap the logger and switch the culture. It won't be hard as I inject it as an ILogger

@304NotModified
Copy link
Member

304NotModified commented Jul 26, 2016

the fancy way to fix is, is writing a LayoutWrapper which switches the Culture.

Something like this:

using System.Globalization;
using System.Threading;
using NLog.Layouts;

namespace NLog.LayoutRenderers.Wrappers
{
    using NLog.Conditions;
    using NLog.Config;


    [LayoutRenderer("SwitchCulture")]
    [AmbientProperty("Culture")]
    [ThreadAgnostic]
    public sealed class SwitchCultureLayoutRendererWrapper : WrapperLayoutRendererBase
    {

        [RequiredParameter]
        public CultureInfo Culture { get; set; }

        /// <summary>
        /// Transforms the output of another layout.
        /// </summary>
        /// <param name="text">Output to be transform.</param>
        /// <returns>Transformed text.</returns>
        protected override string Transform(string text)
        {
            return text;
        }

        /// <summary>
        /// Renders the inner layout contents.
        /// </summary>
        /// <param name="logEvent">The log event.</param>
        /// <returns>
        /// Contents of inner layout.
        /// </returns>
        protected override string RenderInner(LogEventInfo logEvent)
        {
            var oldCulture = Thread.CurrentThread.CurrentUICulture;
            Thread.CurrentThread.CurrentUICulture = Culture;
            //maybe also a new thread ? 
            try
            {
                var value = base.RenderInner(logEvent);
                return value;
            }
            finally
            {
                Thread.CurrentThread.CurrentUICulture = oldCulture;
            }
        }
    }
}

@srollinet
Copy link

@304NotModified Okay, I like this idea because it doesn't involve too much code :)

But I am probably missing something. I have the following exception: "LayoutRenderer cannot be found: 'InvariantCulture'"

using System.Globalization;
using System.Threading;
using NLog.Config;

// ReSharper disable once CheckNamespace
namespace NLog.LayoutRenderers.Wrappers //Is it mandatory to use this namespace?
{
    [LayoutRenderer("InvariantCulture")]
    [ThreadAgnostic]
    public sealed class InvariantCultureLayoutRendererWrapper : WrapperLayoutRendererBase
    {
        protected override string Transform(string text)
        {
            return text;
        }

        protected override string RenderInner(LogEventInfo logEvent)
        {
            var currentCulture = Thread.CurrentThread.CurrentUICulture;
            try
            {
                Thread.CurrentThread.CurrentUICulture = CultureInfo.InvariantCulture;
                var value = base.RenderInner(logEvent);
                return value;
            }
            finally
            {
                Thread.CurrentThread.CurrentUICulture = currentCulture;
            }
        }
    }
}

and the config

<?xml version="1.0" encoding="utf-8" ?>
<nlog xmlns="http://www.nlog-project.org/schemas/NLog.xsd" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" useInvariantCulture="true"  throwExceptions="true">

  <targets>
    <target name="console" xsi:type="ColoredConsole" />
    <target name="logfile" xsi:type="File"
            layout="${longdate}|${level:uppercase=true}|${logger}|${message}|${InvariantCulture:${exception:format=tostring}}"
            archiveNumbering="Date"
            archiveEvery="Day"
            archiveDateFormat="yyyyMMdd"/>
  </targets>

  <rules>... </rules>

</nlog>

@304NotModified
Copy link
Member

304NotModified commented Jul 26, 2016

You need to register it manually or creating an assembly.

e.g.

  ConfigurationItemFactory.Default.LayoutRenderers
          .RegisterDefinition("hello-universe", typeof(MyNamespace.HelloUniverseLayoutRenderer

See http://nlog-project.org/2015/06/30/extending-nlog-is-easy.html

PS; no the namespace is not mandatory

@srollinet
Copy link

Works like a charm. Thanks for your help

@304NotModified
Copy link
Member

Glad to hear!

@Peter-Optiway
Copy link

This should be part of NLog!

@304NotModified
Copy link
Member

we don't like to change the global "Thread.CurrentThread" in NLog. Sounds like a bad idea, so not supported out-of-the-box.

@GibbOne
Copy link

GibbOne commented Feb 22, 2021

Thx for library and suggested solution of the problem.

@304NotModified
Are you sure that it's correct to link logging culture to Thread culture?
Maybe globalization concern should be managed on some other place on NLog?
Am I wrong?
Thx

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants