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 ServeDir infallible #283

Merged
merged 7 commits into from
Dec 2, 2022
Merged

Make ServeDir infallible #283

merged 7 commits into from
Dec 2, 2022

Conversation

davidpdrsn
Copy link
Member

Motivation

Previously ServeDir's error type was io::Error. That means users using ServeDir with axum must use .handle_error() to convert any IO errors into responses:

Router::new().nest(
    "/assets",
    get_service(ServeDir::new(".")).handle_error(handle_error),
);

async fn handle_error(_err: io::Error) -> impl IntoResponse {
    (StatusCode::INTERNAL_SERVER_ERROR, "Something went wrong...")
}

Since ServeDir already maps io::ErrorKind::NotFound and io::ErrorKind::PermissionDenied there isn't much you do for error besides returning StatusCode::INTERNAL_SERVER_ERROR.

That is tedious to do and so I think we should make ServeDir handle the error internally and change its error type to Infallible. That would make the axum integration really simple:

Router::new().nest("/", ServeDir::new("."));

Solution

  • Change <ServeDir as Service<_>>::Error to Infallible
  • Convert any errors that happen to StatusCode::INTERNAL_SERVER_ERROR and log the error
  • Add ServeDir::try_call which returns a future that maintains the error. That way users can still customize the error if they want.

The same was done for ServeFile as its just a wrapper around ServeDir.

Note that this is a breaking change. We shipped the last major 3 months ago so probably fine to do another one but we should consider if there are any other breaking changes we wanna make.

Copy link
Collaborator

@Nehliin Nehliin left a comment

Choose a reason for hiding this comment

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

Nice! this should make things more consistent

tower-http/src/services/fs/serve_dir/mod.rs Show resolved Hide resolved
@davidpdrsn davidpdrsn added the breaking change A PR that makes a breaking change. label Dec 2, 2022
@davidpdrsn davidpdrsn added this to the 0.4 milestone Dec 2, 2022
@davidpdrsn davidpdrsn merged commit f8743bf into master Dec 2, 2022
@davidpdrsn davidpdrsn deleted the serve-dir-infallible branch December 2, 2022 13:17
leotaku added a commit to leotaku/tower-livereload that referenced this pull request Feb 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change A PR that makes a breaking change.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants