Skip to content

JsonConsoleFormatter: keep escaped line breaks for Exceptions #83726#84972

Merged
tarekgh merged 2 commits intodotnet:mainfrom
daberni:jsonconsoleformatter-linebreaks
Apr 20, 2023
Merged

JsonConsoleFormatter: keep escaped line breaks for Exceptions #83726#84972
tarekgh merged 2 commits intodotnet:mainfrom
daberni:jsonconsoleformatter-linebreaks

Conversation

@daberni
Copy link
Contributor

@daberni daberni commented Apr 18, 2023

Fixes #83726

Linebreaks of Exceptions no longer get removed in JsonConsoleFormatter, but instead get escaped by Utf8JsonWriter, so original format can be restored by log viewer.

@daberni

This comment was marked as resolved.

Sign in to view
@build-analysis build-analysis bot mentioned this pull request Apr 18, 2023
Open
2 tasks
@ghost
Copy link

ghost commented Apr 20, 2023

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Issue Details

Fixes #83726

Linebreaks of Exceptions no longer get removed in JsonConsoleFormatter, but instead get escaped by Utf8JsonWriter, so original format can be restored by log viewer.

Author: daberni
Assignees: -
Labels:

area-Extensions-Logging
Milestone: -

@tarekgh
Copy link
Member

tarekgh commented Apr 20, 2023 •
edited
Loading

And we need to add a new test for this change.

[Edit]
I am seeing a couple of tests are failing, so maybe we need to just update these tests.

@tarekgh tarekgh added this to the 8.0.0 milestone Apr 20, 2023
tarekgh
tarekgh requested changes Apr 20, 2023
View reviewed changes
Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to change the failed test to reflect this change.

@ghost ghost added the needs-author-action An issue or pull request that requires more info or actions from the author. label Apr 20, 2023
@ghost ghost removed the needs-author-action An issue or pull request that requires more info or actions from the author. label Apr 20, 2023
@tarekgh
Copy link
Member

tarekgh commented Apr 20, 2023 

            .Replace(Environment.NewLine, newLineReplacement);

Does this line needed anymore?

Refers to: src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/JsonConsoleFormatterTests.cs:534 in 0f22e62. [](commit_id = 0f22e62, deletion_comment = False)

@daberni
Copy link
Contributor Author

daberni commented Apr 20, 2023 •
edited
Loading 

            .Replace(Environment.NewLine, newLineReplacement);

Does this line needed anymore?

Refers to: src/libraries/Microsoft.Extensions.Logging.Console/tests/Microsoft.Extensions.Logging.Console.Tests/JsonConsoleFormatterTests.cs:534 in 0f22e62. [](commit_id = 0f22e62, deletion_comment = False)

This performs the escaping of the newline characters in the test case (to get a proper expected value). Wheras this is automatically handled by Utf8JsonWriter in main srcset. So \r\n gets \\r\\n or \n gets \\n

@tarekgh tarekgh merged commit 02ddff3 into dotnet:main Apr 20, 2023
@daberni daberni deleted the jsonconsoleformatter-linebreaks branch April 21, 2023 05:25
@ghost ghost locked as resolved and limited conversation to collaborators May 21, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@tarekgh tarekgh tarekgh approved these changes

Assignees

No one assigned

Labels

area-Extensions-Logging

Projects

None yet

Milestone

8.0.0

Development

Successfully merging this pull request may close these issues.

JsonConsoleFormatter: (escaped) line breaks of exception stacktraces should be kept

3 participants

@daberni @tarekgh @jeffhandley