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: log multi-line error messages on one line. #595

Merged
merged 2 commits into from
Jan 24, 2019
Merged

Conversation

fisx
Copy link
Contributor

@fisx fisx commented Jan 24, 2019

The proposed fix is very simple, and arguably not the most readable, but it solve the problem of two error messages interleaving each other by dropping all newlines.

Alternatives:

  • replace newline with space, or --, or something else.
  • use netstrings
  • ?

@fisx fisx requested a review from jschaul January 24, 2019 12:34
@@ -195,7 +195,7 @@ catchErrors l m app req k =
errorResponse ex = do
er <- runHandlers ex errorHandlers
when (statusCode (Error.code er) >= 500) $
logIO l Log.Error (Just req) (show ex)
logIO l Log.Error (Just req) (filter (/= '\n') $ show ex)
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's replace \n with a space instead.

(this way, if there is some exotic unicode variant of newline,
we hopefully catch it as well without even knowing.)
@fisx fisx merged commit d0e50bd into develop Jan 24, 2019
@fisx fisx deleted the fisx-500-logging branch January 24, 2019 14:00
@fisx fisx mentioned this pull request Jan 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants