diff --git a/CHANGELOG.md b/CHANGELOG.md index 03acbefe..3f6d8955 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +### Breaking changes + +- fix(actix): capture only server errors ([#877](https://github.com/getsentry/sentry-rust/pull/877)) + - The Actix integration now properly honors the `capture_server_errors` option (enabled by default), capturing errors returned by middleware only if they are server errors (HTTP status code 5xx). + - Previously, if a middleware were to process the request after the Sentry middleware and return an error, our middleware would always capture it and send it to Sentry, regardless if it was a client, server or some other kind of error. + - With this change, we capture errors returned by middleware only if those errors can be classified as server errors. + - There is no change in behavior when it comes to errors returned by services, in which case the Sentry middleware only captures server errors exclusively. + ### Features - feat(core): add Response context ([#874](https://github.com/getsentry/sentry-rust/pull/874)) by @lcian diff --git a/sentry-actix/src/lib.rs b/sentry-actix/src/lib.rs index ede7d749..19fa4394 100644 --- a/sentry-actix/src/lib.rs +++ b/sentry-actix/src/lib.rs @@ -351,7 +351,9 @@ where let mut res: Self::Response = match fut.await { Ok(res) => res, Err(e) => { - if inner.capture_server_errors { + // Errors returned by middleware, and possibly other lower level errors + if inner.capture_server_errors && e.error_response().status().is_server_error() + { hub.capture_error(&e); } @@ -474,6 +476,7 @@ fn process_event(mut event: Event<'static>, request: &Request) -> Event<'static> mod tests { use std::io; + use actix_web::body::BoxBody; use actix_web::test::{call_service, init_service, TestRequest}; use actix_web::{get, web, App, HttpRequest, HttpResponse}; use futures::executor::block_on; @@ -622,9 +625,9 @@ mod tests { } } - /// Ensures client errors (4xx) are not captured. + /// Ensures client errors (4xx) returned by service are not captured. #[actix_web::test] - async fn test_client_errors_discarded() { + async fn test_service_client_errors_discarded() { let events = sentry::test::with_captured_events(|| { block_on(async { let service = HttpResponse::NotFound; @@ -645,6 +648,66 @@ mod tests { assert!(events.is_empty()); } + /// Ensures client errors (4xx) returned by middleware are not captured. + #[actix_web::test] + async fn test_middleware_client_errors_discarded() { + let events = sentry::test::with_captured_events(|| { + block_on(async { + async fn hello_world() -> HttpResponse { + HttpResponse::Ok().body("Hello, world!") + } + + let app = init_service( + App::new() + .wrap_fn(|_, _| async { + Err(actix_web::error::ErrorNotFound("Not found")) + as Result, _> + }) + .wrap(Sentry::builder().with_hub(Hub::current()).finish()) + .service(web::resource("/test").to(hello_world)), + ) + .await; + + let req = TestRequest::get().uri("/test").to_request(); + let res = app.call(req).await; + assert!(res.is_err()); + assert!(res.unwrap_err().error_response().status().is_client_error()); + }) + }); + + assert!(events.is_empty()); + } + + /// Ensures server errors (5xx) returned by middleware are captured. + #[actix_web::test] + async fn test_middleware_server_errors_captured() { + let events = sentry::test::with_captured_events(|| { + block_on(async { + async fn hello_world() -> HttpResponse { + HttpResponse::Ok().body("Hello, world!") + } + + let app = init_service( + App::new() + .wrap_fn(|_, _| async { + Err(actix_web::error::ErrorInternalServerError("Server error")) + as Result, _> + }) + .wrap(Sentry::builder().with_hub(Hub::current()).finish()) + .service(web::resource("/test").to(hello_world)), + ) + .await; + + let req = TestRequest::get().uri("/test").to_request(); + let res = app.call(req).await; + assert!(res.is_err()); + assert!(res.unwrap_err().error_response().status().is_server_error()); + }) + }); + + assert_eq!(events.len(), 1); + } + /// Ensures transaction name can be overridden in handler scope. #[actix_web::test] async fn test_override_transaction_name() {