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 a problem of raw option with handler #136

Merged
merged 2 commits into from
Oct 27, 2019

Conversation

Writtic
Copy link
Contributor

@Writtic Writtic commented Aug 26, 2019

  • Using raw option with handler, the message is cutted last 1 length

When I test my application with loguru, I found something weird.

logger.add(
        sink=LogstashHandler(host, port, "my-app", tags),
        format=LOGSTASH_FORMAT,
        level=level
    )
logger.opt(raw=True).error("hello")

Result: {... 'message': 'hell' ...}

As you could see, If I use handler with raw option, I couldn't get full message on the log.
So I tried to find this solution then somewhat dealt with it with test code.

- Using raw option with handler, the message is cutted last 1 length
if self._is_formatter_dynamic:
formatted = message
else:
if hasattr(self._writer, "read"):
Copy link
Contributor Author

@Writtic Writtic Aug 26, 2019

Choose a reason for hiding this comment

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

Need to check other conditions of self._writer attributes.

@Delgan
Copy link
Owner

Delgan commented Aug 26, 2019

Awesome, thanks for reporting and investigating this issue!

So, to explain what is happening, one should look here:

loguru/loguru/_logger.py

Lines 716 to 717 in 9250317

if not is_formatter_dynamic:
message = message[:-1]

Why am I chopping the last char of the message? Because Loguru automatically adds "\n" at the end of the log messages:

formatter = format + "\n{exception}"

However, standard logging does the same: Lib/logging/__init__.py#L1084

In order to prevent the message to be displayed with two trailing "\n", I remove the one added by Loguru, but only if the format is not a function (because if it is the case, Loguru does not automatically add the trailing "\n").

The bug occurs because the trailing "\n" is not added by Loguru if raw=True. In such case, there is no "\n" to remove, so the last character is wrongly chopped instead.

Here is a small code snippet to play with:

import logging
import sys
from loguru import logger

logger.remove()
logger.add(logging.StreamHandler(sys.stderr), format="{level.no} {message} [Not Chopped]")

logger.error("Allo?")
logger.opt(raw=True).error("hello")

Your solution may work but I would like to avoid having to dynamically check for attributes at run-time in emit(). Actually, there is a very easy solution: remove if not is_formatter_dynamic: message = message[:-1] and replace it with sink.terminator = "". I did not implement this solution is the first time because I wanted not to mutate the sink object, but this is maybe the best solution considering there is no other easy workaround. 😕

@Writtic
Copy link
Contributor Author

Writtic commented Aug 27, 2019

Thank you for very specific and kind replying!
I'm glad to hear the reason of the problem and what your idea about my request is.
I also hoped to avoid amending object level, but I'm not familiar with the project, so I didn't come up with the better solution.

@Delgan Delgan merged commit 1a977f6 into Delgan:master Oct 27, 2019
@Delgan
Copy link
Owner

Delgan commented Oct 27, 2019

I finally fixed this issue in 49c5560. The workaround was quite simple, I just needed to specialize "\n" terminator for both the format and the exception formatter (due to implementation details in logging.Handler). It worries me not to have thought about this sooner... 😁

Anyway, I merged the new unit tests you added, thanks! 😃

@Writtic Writtic deleted the handler_with_raw branch October 27, 2019 09:04
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.

2 participants