Unexpected middleware error behaviour - needs explanation / documentation?
- 主要语言
- Rust
- 星标
- 24.8k
- 派生
- 1.9k
- 平均合并
- 23 小时 10 分钟
- 30 天内合并 PR
- 26
描述
When implementing middleware I discovered various things which seemed confusing:
When we call the underlying service `self.service.call(req)` it returns a result:
As far as I can tell this service call will be Ok even if the route being called returned an error:
```
#[get("/broken_route")]
async fn broken_route() -> Result {
Err(std::io::Error::new(ErrorKind::Other, "500").into())
}
...
async fn test() {
let app = test::init_service(App::new().service(broken_route).wrap(Assert {})).await;
let req = test::TestRequest::get().uri("/broken_route").to_request();
let resp = test::call_service(&app, req).await;
assert_eq!(resp.status(), http::StatusCode::INTERNAL_SERVER_ERROR);
}
...
fn call(&self, req: ServiceRequest) -> Self::Future {
let service = self.service.call(req);
Box::pin(async move {
let result = service.await;
println!("unwrapping service call");
Ok(result.unwrap())
})
}
...
running 1 test
unwrapping service call:
test tests::test ... ok
```
But the service.call() can fail if another middleware in the chain fails:
E.g.
```
let app = test::init_service(App::new().service(broken_route).wrap(Broken {}).wrap(Assert {})).await;
...
running 1 test
unwrapping service call:
thread 'tests::test2' panicked at 'called `Result::unwrap()` on an `Err` value: Custom { kind: Other, error: "oh no!" }', src\main.rs:146:23
```
Now this leads to a bug when using the `ErrorHandlers` middleware:
```
fn add_error_header(
mut res: ServiceResponse,
) -> Result, actix_web::Error> {
res.response_mut().headers_mut().insert(
header::CONTENT_TYPE,
header::HeaderValue::from_static("Error"),
);
Ok(ErrorHandlerResponse::Response(res.map_into_left_body()))
}
...
async fn test() {
let srv = actix_test::start(|| App::new()
.wrap(Broken {})
.wrap(
ErrorHandlers::new()
.handler(StatusCode::INTERNAL_SERVER_ERROR, add_error_header),
)
.service(broken_route));
let resp = reqwest::get(srv.url("/broken_route")).await.unwrap();
assert_eq!(resp.status(), http::StatusCode::INTERNAL_SERVER_ERROR);
// Fails here, ErrorHandlers did not apply
assert_eq!(resp.headers().get(header::CONTENT_TYPE).unwrap(), "Error");
}
```
The Broken middleware is returning a ResponseError which will cause a 500 error, but the ErrorHandler middleware [returns early](https://github.com/actix/actix-web/blob/master/actix-web/src/middleware/err_handlers.rs#L157) because it got an error when calling the underlying middleware, so it doesn't run.
## Possible Solution
Please can we have better documentation for how to handle errors when writing middleware:
1. Document when a service call can fail
2. Document that presumably a middleware should always return a ServiceResponse or risk breaking other middlewares up the chain (e.g. ErrorHandlers, Loggers, Cors etc.)?
I do wonder if a middleware should even be allowed to return an error / if the current pattern just encourages mistakes?
## See Also
https://github.com/actix/actix-extras/pull/128
https://github.com/actix/actix-web/issues/1051
## Your Environment
- Rust Version: 1.59
- Actix Web Version: 4.0.1
贡献指南
评估
这个 Issue 还没有评估数据。