Skip to content

reset_middleware() doesn't prevent middleware from running for route #850

Open

Description

@0b10011

reset_middleware() doesn't affect which middleware is ran for a route. With the following code, I'd expect a 200 response with the text "Success!". Instead, the log shows:

thread 'async-std/runtime' panicked at 'Middleware panicked.', src\main.rs:25:9

cargo.toml

[package]
name = "tide-middleware-test"
version = "0.1.0"
edition = "2018"
publish = false

[dependencies]
async-std = { version = "1.6.0", features = ["attributes"] }
tide = "0.16.0"

src/main.rs

use tide::{Middleware, Next, Request, Response, Result};

#[async_std::main]
async fn main() -> std::result::Result<(), Box<dyn std::error::Error>> {
    let mut app = tide::new();

    app.with(PanicMiddleware {});

    app.at("/").reset_middleware().get(page);

    app.listen("0.0.0.0:80").await?;

    Ok(())
}

async fn page(_request: Request<()>) -> tide::Result {
    Ok("Success!".into())
}

struct PanicMiddleware {}

#[tide::utils::async_trait]
impl<State: Clone + Send + Sync + 'static> Middleware<State> for PanicMiddleware {
    async fn handle(&self, _request: Request<State>, _next: Next<'_, State>) -> Result<Response> {
        panic!("Middleware panicked.");
    }
}

Activity

  1. 0b10011 commented on Sep 20, 2021

    @0b10011
    Author

    It looks like this is specifically due to adding the middleware to Server instead of Route. Here's a test for this issue:

    #[async_std::test]
    async fn app_middleware_reset() -> tide::Result<()> {
        let mut app = tide::new();
        app.with(TestMiddleware::with_header_name("X-Root", "root"));
        app.at("/foo")
            .reset_middleware()
            .with(TestMiddleware::with_header_name("X-Foo", "foo"))
            .get(echo_path);
    
        let res = app.get("/foo").await?;
        assert!(res.header("X-Root").is_none());
        assert_eq!(res["X-Foo"], "foo");
        Ok(())
    }
  2. u5surf commented on Oct 9, 2021

    @u5surf
    Contributor

    @jnicklas @0b10011
    Hi, I have a simple resolution for this issue.

    It looks like this is specifically due to adding the middleware to Server instead of Route. Here's a test for this issue:

    For example, implement server.reset_middleware instead of route.reset_middleware, we are possible to reset middleware which server has.

    For usages:

    #[async_std::test]
    async fn app_middleware_reset() -> tide::Result<()> {
        let mut app = tide::new();
        app.with(TestMiddleware::with_header_name("X-Root", "root"));
        app.reset_middleware(); // reset this!
        app.at("/foo")
            .with(TestMiddleware::with_header_name("X-Foo", "foo"))
            .get(echo_path);
    
        let res = app.get("/foo").await?;
        assert!(res.header("X-Root").is_none());
        assert_eq!(res["X-Foo"], "foo");
        Ok(())
    }
    

    According to implementation of server.reset_middleware in my local, it seems to be passed this tests.

    @yoshuawuyts
    Can I work on this issue if you approved to resolve this way?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions