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

Make .catch() context manager async compatible #1084

Open
Jonas1312 opened this issue Feb 17, 2024 · 2 comments
Open

Make .catch() context manager async compatible #1084

Jonas1312 opened this issue Feb 17, 2024 · 2 comments
Labels
enhancement Improvement to an already existing feature

Comments

@Jonas1312
Copy link

Jonas1312 commented Feb 17, 2024

Hi,

First, thanks for a lot for loguru :)

I have a piece of code like this:

import httpx
from loguru import logger as loguru_logger

@app.get("/")  # some fastapi route
async def my_route(foo, bar):
    logger = loguru_logger.bind(foo=foo, bar=bar)  # add some context to my logger
    with logger.catch():  # first context manager
        async with httpx.AsyncClient() as client:  # second context manager
            r = await client.get('https://www.example.com/')
            # do stuff...

In python, it's possible to chain context managers:

with A() as X, B() as Y, C() as Z:
    do_something()

But I don't think it's possible to mix async and sync context managers.

So I wonder if we could make the class Catcher async compatible? I think that it would require implementing __aenter__ and __aexit__

This feature would allow this:

import httpx
from loguru import logger as loguru_logger

@app.get("/")  # some fastapi route
async def my_route(foo, bar):
    logger = loguru_logger.bind(foo=foo, bar=bar)  # add some context to my logger
    async with logger.catch(), httpx.AsyncClient() as client:  # all in one line
        r = await client.get('https://www.example.com/')
        # do stuff...

Thanks

@Delgan
Copy link
Owner

Delgan commented Feb 17, 2024

Hey @Jonas1312. Thanks for the suggestion, it should be relatively easy to implement.

However, I'm wondering if it's good practice to make logger.catch() compatible with async, when it has nothing to do with async (it's just syntactic sugar for try / except)?

I suppose it shouldn't be too much of a concern, but I'd be interested to know of other libraries or built-ins context managers that have such behavior.

@Delgan Delgan added the enhancement Improvement to an already existing feature label Feb 17, 2024
@Jonas1312
Copy link
Author

Jonas1312 commented Feb 17, 2024

Yes I guess it would be relatively easy to implement:

  • __aenter__ would be empty
  • __aexit__ would call __exit__, followed by a await logger.complete() maybe?

For now, I'm making loguru catch compatible with async like this:

@asynccontextmanager
async def catch_error(logger: "loguru.Logger") -> AsyncGenerator[None, None]:
    with logger.catch(reraise=True):
        yield

logger = loguru_logger.bind(foo=foo, bar=bar)  # add some context to my logger
async with catch_error(logger), httpx.AsyncClient() as client:  # all in one line
    r = await client.get('https://www.example.com/')
    # do stuff...

But that would be cleaner if loguru could do it natively.

There’s nothing dangerous about calling synchronous functions inside asynchronous ones (except blocking the event loop...), so it should be safe

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an already existing feature
Projects
None yet
Development

No branches or pull requests

2 participants