Skip to content

Commit 556cc61

Browse files
authored
errors: Support contextual error handling strategies (#1246)
Currently `app_core::errors` handles synthesis of all error responses. This means that this module needs to know about all possible error types. To work around this, we wrap some errors with a special `HttpError` type so this module can create a synthetic response. Furthermore, this means that we can't handle errors differently in inbound/outbound contexts. This change modies the `error::respond` module with a new strategy trait--`HttpRescue`--that is responsible for producing a `SyntheticHttpResponse` if the error can be handled and returning an error if it should not be handled gracefully. This change enables us to perform narrower error handling in more places in the stack--for example, so that some error responses are included in response metrics. This change modifies HTTP/1 error responses to include a `connection: close` header when the server-side connection is being torn down.
1 parent 47a5757 commit 556cc61

File tree

18 files changed

+481
-322
lines changed

18 files changed

+481
-322
lines changed

linkerd/app/admin/src/stack.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,10 +81,12 @@ impl Config {
8181
let admin = crate::server::Admin::new(report, ready, shutdown, trace);
8282
let admin = svc::stack(move |_| admin.clone())
8383
.push(metrics.proxy.http_endpoint.to_layer::<classify::Response, _, Http>())
84-
.push(metrics.http_errors.to_layer())
8584
.push_on_service(
8685
svc::layers()
87-
.push(errors::respond::layer())
86+
.push(errors::NewRespond::layer(|error: Error| -> Result<_> {
87+
tracing::warn!(%error, "Unexpected error");
88+
Ok(errors::SyntheticHttpResponse::unexpected_error())
89+
}))
8890
.push(http::BoxResponse::layer()),
8991
)
9092
.push(http::NewServeHttp::layer(Default::default(), drain.clone()))

linkerd/app/core/src/errors/mod.rs

Lines changed: 10 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,44 +1,20 @@
11
pub mod respond;
22

3-
use linkerd_error::Error;
3+
pub use self::respond::{HttpRescue, NewRespond, SyntheticHttpResponse};
4+
pub use linkerd_proxy_http::h2::H2Error;
45
pub use linkerd_timeout::{FailFastError, ResponseTimeout};
56
use thiserror::Error;
67

78
#[derive(Debug, Error)]
89
#[error("connect timed out after {0:?}")]
9-
pub(crate) struct ConnectTimeout(pub std::time::Duration);
10+
pub struct ConnectTimeout(pub(crate) std::time::Duration);
1011

11-
#[derive(Debug, Error)]
12-
#[error("{source}")]
13-
pub struct HttpError {
14-
#[source]
15-
source: Error,
16-
http_status: http::StatusCode,
17-
grpc_status: tonic::Code,
18-
}
19-
20-
impl HttpError {
21-
pub fn bad_request(source: impl Into<Error>) -> Self {
22-
Self {
23-
source: source.into(),
24-
http_status: http::StatusCode::BAD_REQUEST,
25-
grpc_status: tonic::Code::InvalidArgument,
26-
}
27-
}
28-
29-
pub fn forbidden(source: impl Into<Error>) -> Self {
30-
Self {
31-
source: source.into(),
32-
http_status: http::StatusCode::FORBIDDEN,
33-
grpc_status: tonic::Code::PermissionDenied,
34-
}
35-
}
36-
37-
pub fn loop_detected(source: impl Into<Error>) -> Self {
38-
Self {
39-
source: source.into(),
40-
http_status: http::StatusCode::LOOP_DETECTED,
41-
grpc_status: tonic::Code::Aborted,
42-
}
12+
/// Obtain the source error at the end of a chain of `Error`s.
13+
pub fn root_cause<'e>(
14+
mut error: &'e (dyn std::error::Error + 'static),
15+
) -> &'e (dyn std::error::Error + 'static) {
16+
while let Some(e) = error.source() {
17+
error = e;
4318
}
19+
error
4420
}

0 commit comments

Comments
 (0)