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

KeyError in string format when curlies { } present #1008

Open
crizCraig opened this issue Oct 18, 2023 · 7 comments
Open

KeyError in string format when curlies { } present #1008

crizCraig opened this issue Oct 18, 2023 · 7 comments

Comments

@crizCraig
Copy link

crizCraig commented Oct 18, 2023

I'm running into the same problem as #767 and used a similar fix, but one that still tries to do the formatting.

So my message is

Retrying timed.<locals>.wrapped_fn in 0.9077886023569136 seconds as it raised Exception: {...foo_json...} => problem parsing resp!

And I basically get KeyError: ...foo_json... where foo_json is some JSON I'm logging with the problematic curlies.

class SafeDict(dict):
    def __missing__(self, key):
        return '{' + key + '}'
        
...
try:
    log_record["message"] = message.format(*args, **kwargs)
except (KeyError, IndexError):
    # KeyError from { missing_key } in message, IndexError from missing positional args.
    try:
        log_record["message"] = message.format_map(SafeDict(kwargs))
    except ValueError:
        log_record["message"] = message

loguru version: 0.7.0

Thanks for the awesome library @Delgan !

@Delgan
Copy link
Owner

Delgan commented Oct 20, 2023

Hi @crizCraig.

Did the SafeDict workaround solves the issue you're facing?

I'm not really sure how I can help you there. When arguments are passed to the logging call, Loguru uses them to format the message. If the arguments were not meant for formatting but the message contains curly braces, Python will complain.

If you need the keyword argument to only populate the extra dict, you might use logger.bind() instead.

I'm planning to disable keyword arguments formatting in the future, though, which should help with the issue you're facing.

@crizCraig
Copy link
Author

crizCraig commented Oct 20, 2023

Thanks @Delgan! Yes, the guardrails above worked.

kwargs are populated by this tenacity retry decorator

@retry(
    stop=stop_after_attempt(10),
    wait=wait_random_exponential(multiplier=1, max=10),
    reraise=True,
    before_sleep=before_sleep_log(log, "ERROR"),  # type: ignore
    after=after_log(log, "ERROR"),  # type: ignore
)

which does this

logger.log(
    log_level,
    f"Retrying {fn_name} " f"in {retry_state.next_action.sleep} seconds as it {verb} {value}.",
    exc_info=local_exc_info,
)

@crizCraig
Copy link
Author

Perhaps the best thing here is for tenacity to not allow setting up retries with a loguru logger. Here's a little prototype of wrapping a loguru logger with std logging that might be better

from loguru import logger as loguru_logging

import logging

logging.basicConfig(format='%(asctime)s %(levelname)s %(message)s', level=logging.DEBUG)

class InterceptHandler(logging.Handler):
    def emit(self, record):
        # Get corresponding Loguru level if it exists
        try:
            level = loguru_logging.level(record.levelname).name
        except ValueError:
            level = record.levelno

        # Find caller from where originated the logged message
        frame, depth = logging.currentframe(), 2
        while frame.f_code.co_filename == logging.__file__:
            frame = frame.f_back
            depth += 1

        loguru_logger = loguru_logging.bind(_depth=depth, _frame=frame)
        loguru_logger.log(level, record.msg)


# get root std logging object
std_root_logger = logging.getLogger()

# Add the interceptor handler to the standard logger
std_root_logger.addHandler(InterceptHandler())
std_root_logger.handlers.pop(0)  # Remove the default handler added by logging.basicConfig


loguru_logging.info("This is from Loguru!")
logging.info("This is from standard logging!")

@Delgan
Copy link
Owner

Delgan commented Oct 23, 2023

Hey @crizCraig. Do you know what happened to the https://github.com/jd/tenacity project? It returns 404. 😬

@crizCraig
Copy link
Author

crizCraig commented Oct 23, 2023 via email

@Delgan
Copy link
Owner

Delgan commented Oct 27, 2023

Thanks for the additional details, @crizCraig.

It seems tenacity expects a logging.logger object, therefore we should not expect the tenacity library to be compatible with Loguru unfortunately.

This issue is similar to another one in which I share a possible workaround, please take a look: #969 (comment)
The idea is to wrap Loguru in a standard logger, and pass this logger to the third-library so that logs can be transparently intercepted by Loguru from the standard logging.

The InterceptHandler implementation is quite cumbersome for now, but it should be better once I implement a logger.bridge() method.

@crizCraig
Copy link
Author

Agreed, thanks @Delgan!

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

No branches or pull requests

2 participants