[go: up one dir, main page]

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

Change Connected::connect_info to return Self #396

Merged
merged 1 commit into from
Oct 19, 2021

Conversation

davidpdrsn
Copy link
Member

I've been thinking that having an associated type probably isn't
necessary. I imagine most users are either using SocketAddr to the
remote connection IP, or writing their own connection struct.

@davidpdrsn davidpdrsn added the C-cleanup Category: PRs that clean code up or issues documenting cleanup. label Oct 19, 2021
I've been thinking that having an associated type probably isn't
necessary. I imagine most users are either using `SocketAddr` to the
remote connection IP, or writing their own connection struct.
Copy link
Member
@jplatte jplatte left a comment

Choose a reason for hiding this comment

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

I'm not familiar with this part of the API but if you're not aware of any users who have a ConnectionInfo type that's not Self this seems like a good change. Why was it an associated type to begin with?

@davidpdrsn
Copy link
Member Author

I believe it was a holdover tonic's Connected trait. Tonic has a full featured server with TLS support so the associated type makes sense in that context but yeah I don't believe its necessary for axum.

@davidpdrsn davidpdrsn merged commit 9fcc884 into main Oct 19, 2021
@davidpdrsn davidpdrsn deleted the simplify-connect-info branch October 19, 2021 21:06
davidpdrsn added a commit that referenced this pull request Nov 2, 2021
- Overall:
  - **fixed:** All known compile time issues are resolved, including those with
    `boxed` and those introduced by Rust 1.56 ([#404])
  - **breaking:** The router's type is now always `Router` regardless of how many routes or
    middleware are applied ([#404])

    This means router types are all always nameable:

    ```rust
    fn my_routes() -> Router {
        Router::new().route(
            "/users",
            post(|| async { "Hello, World!" }),
        )
    }
    ```
  - **breaking:** Added feature flags for HTTP1 and JSON. This enables removing a
    few dependencies if your app only uses HTTP2 or doesn't use JSON. Its only a
    breaking change if you depend on axum with `default_features = false`. ([#286])
  - **breaking:** `Route::boxed` and `BoxRoute` have been removed as they're no longer
    necessary ([#404])
  - **breaking:** `Nested`, `Or` types are now private. They no longer had to be
    public because `Router` is internally boxed ([#404])
  - **breaking:** Remove `routing::Layered` as it didn't actually do anything and
    thus wasn't necessary
  - **breaking:** Vendor `AddExtensionLayer` and `AddExtension` to reduce public
    dependencies
  - **breaking:** `body::BoxBody` is now a type alias for
    `http_body::combinators::UnsyncBoxBody` and thus is no longer `Sync`. This
    is because bodies are streams and requiring streams to be `Sync` is
    unnecessary.
  - **added:** Implement `IntoResponse` for `http_body::combinators::UnsyncBoxBody`.
  - **added:** Add `Handler::into_make_service` for serving a handler without a
    `Router`.
  - **added:** Add `Handler::into_make_service_with_connect_info` for serving a
    handler without a `Router`, and storing info about the incoming connection.
  - **breaking:** axum's minimum support rust version is not 1.54
- Routing:
  - Big internal refactoring of routing leading to several improvements ([#363])
    - **added:** Wildcard routes like `.route("/api/users/*rest", service)` are now supported.
    - **fixed:** The order routes are added in no longer matters.
    - **fixed:** Adding a conflicting route will now cause a panic instead of silently making
      a route unreachable.
    - **fixed:** Route matching is faster as number of routes increase.
  - **fixed:** Correctly handle trailing slashes in routes:
    - If a route with a trailing slash exists and a request without a trailing
      slash is received, axum will send a 301 redirection to the route with the
      trailing slash.
    - Or vice versa if a route without a trailing slash exists and a request
      with a trailing slash is received.
    - This can be overridden by explicitly defining two routes: One with and one
      without trailing a slash.
  - **breaking:** Method routing for handlers have been moved from `axum::handler`
    to `axum::routing`. So `axum::handler::get` now lives at `axum::routing::get`
    ([#405])
  - **breaking:** Method routing for services have been moved from `axum::service`
    to `axum::routing`. So `axum::service::get` now lives at, etc.
    `axum::routing::service_method_routing::get`, etc. ([#405])
  - **breaking:** `Router::or` renamed to `Router::merge` and will now panic on
    overlapping routes. It now only accepts `Router`s and not general `Service`s.
    Use `Router::fallback` for adding fallback routes ([#408])
  - **added:** `Router::fallback` for adding handlers for request that didn't
    match any routes. `Router::fallback` must be use instead of `nest("/", _)` ([#408])
  - **breaking:** `EmptyRouter` has been renamed to `MethodNotAllowed` as its only
    used in method routers and not in path routers (`Router`)
  - **breaking:** Remove support for routing based on the `CONNECT` method. An
    example of combining axum with and HTTP proxy can be found [here][proxy] ([#428])
- Extractors:
  - **fixed:** Expand accepted content types for JSON requests ([#378])
  - **fixed:** Support deserializing `i128` and `u128` in `extract::Path`
  - **breaking:** Automatically do percent decoding in `extract::Path`
    ([#272])
  - **breaking:** Change `Connected::connect_info` to return `Self` and remove
    the associated type `ConnectInfo` ([#396])
  - **added:** Add `extract::MatchedPath` for accessing path in router that
    matched the request ([#412])
- Error handling:
  - **breaking:** Simplify error handling model ([#402]):
    - All services part of the router are now required to be infallible.
    - Error handling utilities have been moved to an `error_handling` module.
    - `Router::check_infallible` has been removed since routers are always
      infallible with the error handling changes.
    - Error handling closures must now handle all errors and thus always return
      something that implements `IntoResponse`.

    With these changes handling errors from fallible middleware is done like so:

    ```rust,no_run
    use axum::{
        routing::get,
        http::StatusCode,
        error_handling::HandleErrorLayer,
        response::IntoResponse,
        Router, BoxError,
    };
    use tower::ServiceBuilder;
    use std::time::Duration;

    let middleware_stack = ServiceBuilder::new()
        // Handle errors from middleware
        //
        // This middleware most be added above any fallible
        // ones if you're using `ServiceBuilder`, due to how ordering works
        .layer(HandleErrorLayer::new(handle_error))
        // Return an error after 30 seconds
        .timeout(Duration::from_secs(30));

    let app = Router::new()
        .route("/", get(|| async { /* ... */ }))
        .layer(middleware_stack);

    fn handle_error(_error: BoxError) -> impl IntoResponse {
        StatusCode::REQUEST_TIMEOUT
    }
    ```

    And handling errors from fallible leaf services is done like so:

    ```rust
    use axum::{
        Router, service,
        body::Body,
        routing::service_method_routing::get,
        response::IntoResponse,
        http::{Request, Response},
        error_handling::HandleErrorExt, // for `.handle_error`
    };
    use std::{io, convert::Infallible};
    use tower::service_fn;

    let app = Router::new()
        .route(
            "/",
            get(service_fn(|_req: Request<Body>| async {
                let contents = tokio::fs::read_to_string("some_file").await?;
                Ok::<_, io::Error>(Response::new(Body::from(contents)))
            }))
            .handle_error(handle_io_error),
        );

    fn handle_io_error(error: io::Error) -> impl IntoResponse {
        // ...
    }
    ```
- Misc:
  - `InvalidWebsocketVersionHeader` has been renamed to `InvalidWebSocketVersionHeader` ([#416])
  - `WebsocketKeyHeaderMissing` has been renamed to `WebSocketKeyHeaderMissing` ([#416])

[#339]: #339
[#286]: #286
[#272]: #272
[#378]: #378
[#363]: #363
[#396]: #396
[#402]: #402
[#404]: #404
[#405]: #405
[#408]: #408
[#412]: #412
[#416]: #416
[#428]: #428
[proxy]: https://github.com/tokio-rs/axum/blob/main/examples/http-proxy/src/main.rs
@davidpdrsn davidpdrsn mentioned this pull request Nov 2, 2021
davidpdrsn added a commit that referenced this pull request Nov 2, 2021
- Overall:
  - **fixed:** All known compile time issues are resolved, including those with
    `boxed` and those introduced by Rust 1.56 ([#404])
  - **breaking:** The router's type is now always `Router` regardless of how many routes or
    middleware are applied ([#404])

    This means router types are all always nameable:

    ```rust
    fn my_routes() -> Router {
        Router::new().route(
            "/users",
            post(|| async { "Hello, World!" }),
        )
    }
    ```
  - **breaking:** Added feature flags for HTTP1 and JSON. This enables removing a
    few dependencies if your app only uses HTTP2 or doesn't use JSON. Its only a
    breaking change if you depend on axum with `default_features = false`. ([#286])
  - **breaking:** `Route::boxed` and `BoxRoute` have been removed as they're no longer
    necessary ([#404])
  - **breaking:** `Nested`, `Or` types are now private. They no longer had to be
    public because `Router` is internally boxed ([#404])
  - **breaking:** Remove `routing::Layered` as it didn't actually do anything and
    thus wasn't necessary
  - **breaking:** Vendor `AddExtensionLayer` and `AddExtension` to reduce public
    dependencies
  - **breaking:** `body::BoxBody` is now a type alias for
    `http_body::combinators::UnsyncBoxBody` and thus is no longer `Sync`. This
    is because bodies are streams and requiring streams to be `Sync` is
    unnecessary.
  - **added:** Implement `IntoResponse` for `http_body::combinators::UnsyncBoxBody`.
  - **added:** Add `Handler::into_make_service` for serving a handler without a
    `Router`.
  - **added:** Add `Handler::into_make_service_with_connect_info` for serving a
    handler without a `Router`, and storing info about the incoming connection.
  - **breaking:** axum's minimum support rust version is not 1.54
- Routing:
  - Big internal refactoring of routing leading to several improvements ([#363])
    - **added:** Wildcard routes like `.route("/api/users/*rest", service)` are now supported.
    - **fixed:** The order routes are added in no longer matters.
    - **fixed:** Adding a conflicting route will now cause a panic instead of silently making
      a route unreachable.
    - **fixed:** Route matching is faster as number of routes increase.
  - **fixed:** Correctly handle trailing slashes in routes:
    - If a route with a trailing slash exists and a request without a trailing
      slash is received, axum will send a 301 redirection to the route with the
      trailing slash.
    - Or vice versa if a route without a trailing slash exists and a request
      with a trailing slash is received.
    - This can be overridden by explicitly defining two routes: One with and one
      without trailing a slash.
  - **breaking:** Method routing for handlers have been moved from `axum::handler`
    to `axum::routing`. So `axum::handler::get` now lives at `axum::routing::get`
    ([#405])
  - **breaking:** Method routing for services have been moved from `axum::service`
    to `axum::routing`. So `axum::service::get` now lives at, etc.
    `axum::routing::service_method_routing::get`, etc. ([#405])
  - **breaking:** `Router::or` renamed to `Router::merge` and will now panic on
    overlapping routes. It now only accepts `Router`s and not general `Service`s.
    Use `Router::fallback` for adding fallback routes ([#408])
  - **added:** `Router::fallback` for adding handlers for request that didn't
    match any routes. `Router::fallback` must be use instead of `nest("/", _)` ([#408])
  - **breaking:** `EmptyRouter` has been renamed to `MethodNotAllowed` as its only
    used in method routers and not in path routers (`Router`)
  - **breaking:** Remove support for routing based on the `CONNECT` method. An
    example of combining axum with and HTTP proxy can be found [here][proxy] ([#428])
- Extractors:
  - **fixed:** Expand accepted content types for JSON requests ([#378])
  - **fixed:** Support deserializing `i128` and `u128` in `extract::Path`
  - **breaking:** Automatically do percent decoding in `extract::Path`
    ([#272])
  - **breaking:** Change `Connected::connect_info` to return `Self` and remove
    the associated type `ConnectInfo` ([#396])
  - **added:** Add `extract::MatchedPath` for accessing path in router that
    matched the request ([#412])
- Error handling:
  - **breaking:** Simplify error handling model ([#402]):
    - All services part of the router are now required to be infallible.
    - Error handling utilities have been moved to an `error_handling` module.
    - `Router::check_infallible` has been removed since routers are always
      infallible with the error handling changes.
    - Error handling closures must now handle all errors and thus always return
      something that implements `IntoResponse`.

    With these changes handling errors from fallible middleware is done like so:

    ```rust,no_run
    use axum::{
        routing::get,
        http::StatusCode,
        error_handling::HandleErrorLayer,
        response::IntoResponse,
        Router, BoxError,
    };
    use tower::ServiceBuilder;
    use std::time::Duration;

    let middleware_stack = ServiceBuilder::new()
        // Handle errors from middleware
        //
        // This middleware most be added above any fallible
        // ones if you're using `ServiceBuilder`, due to how ordering works
        .layer(HandleErrorLayer::new(handle_error))
        // Return an error after 30 seconds
        .timeout(Duration::from_secs(30));

    let app = Router::new()
        .route("/", get(|| async { /* ... */ }))
        .layer(middleware_stack);

    fn handle_error(_error: BoxError) -> impl IntoResponse {
        StatusCode::REQUEST_TIMEOUT
    }
    ```

    And handling errors from fallible leaf services is done like so:

    ```rust
    use axum::{
        Router, service,
        body::Body,
        routing::service_method_routing::get,
        response::IntoResponse,
        http::{Request, Response},
        error_handling::HandleErrorExt, // for `.handle_error`
    };
    use std::{io, convert::Infallible};
    use tower::service_fn;

    let app = Router::new()
        .route(
            "/",
            get(service_fn(|_req: Request<Body>| async {
                let contents = tokio::fs::read_to_string("some_file").await?;
                Ok::<_, io::Error>(Response::new(Body::from(contents)))
            }))
            .handle_error(handle_io_error),
        );

    fn handle_io_error(error: io::Error) -> impl IntoResponse {
        // ...
    }
    ```
- Misc:
  - `InvalidWebsocketVersionHeader` has been renamed to `InvalidWebSocketVersionHeader` ([#416])
  - `WebsocketKeyHeaderMissing` has been renamed to `WebSocketKeyHeaderMissing` ([#416])

[#339]: #339
[#286]: #286
[#272]: #272
[#378]: #378
[#363]: #363
[#396]: #396
[#402]: #402
[#404]: #404
[#405]: #405
[#408]: #408
[#412]: #412
[#416]: #416
[#428]: #428
[proxy]: https://github.com/tokio-rs/axum/blob/main/examples/http-proxy/src/main.rs
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-cleanup Category: PRs that clean code up or issues documenting cleanup.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants