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

Don't log error when blocking consume #47

Merged
merged 3 commits into from
Sep 13, 2024
Merged

Don't log error when blocking consume #47

merged 3 commits into from
Sep 13, 2024

Conversation

spuun
Copy link
Member

@spuun spuun commented Sep 3, 2024

The exception is always re-raised in the calling fiber and must be handled by the app, therefore it doesn't need to be logged as an error by the library. Logging it as error introduces a "scary" backtrace and error log line in the app log even though the exception is handled.

This PR will change to only log if it's a non-blocking consume.

@snichme
Copy link
Member

snichme commented Sep 3, 2024

Why do we log it at all if it's raised to the app using the library? Isn't is better to allow the user to handle it? It could be that the user doesn't want it to log or send the log to a different source that isn't stdout

@spuun
Copy link
Member Author

spuun commented Sep 3, 2024

Why do we log it at all if it's raised to the app using the library? Isn't is better to allow the user to handle it? It could be that the user doesn't want it to log or send the log to a different source that isn't stdout

Yeah, but I thought debug could make somewhat sense, but can't really motivate it 🙃

@snichme
Copy link
Member

snichme commented Sep 3, 2024

I dont know whats best, but personally I prefer libraries not logging anything themself.

@carlhoerberg
Copy link
Member

It's only raised if basic_consume is blocking, when the consuming is done in the background fibers will die silently with this change

@spuun
Copy link
Member Author

spuun commented Sep 4, 2024

It's only raised if basic_consume is blocking, when the consuming is done in the background fibers will die silently with this change

So add a condition to log if blocking or not?

@spuun spuun changed the title Log with debug instead of error Don't log error when blocking consume Sep 13, 2024
@spuun spuun merged commit 8c5077e into main Sep 13, 2024
3 checks passed
@spuun spuun deleted the less-scary-logging branch September 13, 2024 08:45
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.

4 participants