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

Improving error handling and logging in the server loop #2916

Conversation

lonestarx1
Copy link

Motivation

First time open-source contributor here 🤗
I have been learning Rust for a few months and loved the language. After covering the book I started looking for an interesting project to contribute to. Since I have been using working with backend framework for a few years in other languages, I came to like Axum. I am looking for a way to be useful to the project, starting in the simplest way possible.
Hence this PR is simply an attempt to improve error visibility.

Solution

  1. Replaced the usage of unwrap_or_else with poll_fn and a match statement to ensure errors are logged and handled appropriately rather than panicking or proceeding without full error context.

  2. Introduced better error logging with the error! macro when service creation fails, improving visibility when something goes wrong during the make_service call.

  3. Added trace logging to capture connection errors for better debugging of networking issues, which would otherwise be difficult to trace.

First timers make plenty of mistakes, I am happy to learn!

Thanks for the good work 👍🏽

@lonestarx1 lonestarx1 closed this Sep 19, 2024
Copy link
Collaborator

@mladedav mladedav left a comment

Choose a reason for hiding this comment

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

Thanks for the effort!

I've just added a comment explaining a bit why the unwrap_or_else is used here and why it's arguably preferable in this case to handling the errors explicitly. Although now I see clippy tried to tell you the same so you might have already figured what is going on here on your own.

@@ -235,18 +235,24 @@ where

let tcp_stream = TokioIo::new(tcp_stream);

poll_fn(|cx| make_service.poll_ready(cx))
.await
.unwrap_or_else(|err| match err {});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is actually fine because the error variant is Infallible, which is a type that cannot be constructed. It's defined as an enum with 0 variants, so there is no way to construct it. That's also partly why the match is here, if anyone adds a variant to that enum for some reason, this would not compile anymore.

Basically we know that the closure never actually runs. The compiler also knows it and can optimize all this code away. When you added the body here, it actually also is unreachable code.

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