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

Fix for #365: Behaviour when logging null arguments #366

Merged
3 commits merged into from
Jun 18, 2014

Conversation

cvanbergen
Copy link
Contributor

Parsing a [null] parameter in for example logger.Info("Testing null: {0}", null); will result in not logging an event. A
System.NullReferenceException would be thrown causing the entire logEvent NOT to be logged. This behavior is different from prior versions. This fix will restore the previous behavior.

Parsing a [null] parameter in for example logger.Info("Testing null:
{0}", null); will result in not logging an event. A
System.NullReferenceException would be thrown causing the entire
logEvent  NOT to be logged. This behavior is different from prior
versions. This fix will restore the previous behavior.
@ghost
Copy link

ghost commented Jun 15, 2014

Looks good, could you create a test for this?

Added unit test  for fix NLog#365
@cvanbergen
Copy link
Contributor Author

@Xharze , test is added.

@jonreis
Copy link

jonreis commented Jun 17, 2014

As pointed out here: #355, it would be better to use:

var logEvent = item as LogEventInfo;
if (logEvent!=null)
{

@cvanbergen
Copy link
Contributor Author

Thank you @jonreis , you are right. I totally overlooked that issue. I changed the code just now.
@Xharze, I think that this PR will resolve both #355 and #365.

@ghost ghost added the bug label Jun 18, 2014
@ghost ghost added this to the 3.0.1 milestone Jun 18, 2014
ghost pushed a commit that referenced this pull request Jun 18, 2014
Fix for #365: Behaviour when logging null arguments
@ghost ghost merged commit cf38425 into NLog:master Jun 18, 2014
@cvanbergen cvanbergen deleted the issue365 branch June 19, 2014 05:38
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug report / Bug fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants