Skip to content

Commit 3e02edc

Browse files
david-perezunexge
authored andcommitted
DEBUG-log server request rejections (#2597)
This commit logs server request rejections at the `DEBUG` level in an operation's `FromRequest` implementation. This commit is analogous to the one in PR #2524 for response rejections. However, request rejections are _not_ errors, so they shouldn't be logged at the `ERROR` level. Indeed, they happen every time the server rejects a malformed request. Prior to this commit, the `RuntimeError::NotAcceptable` variant was the only `RuntimeError` variant that was manually constructed. This commit makes it so that it now results from a conversion from a new `RequestRejection::NotAcceptable` variant. We now leverage `futures_util::future::TryFutureExt::map` to map a future that uses `RequestRejection` as its error into a future that uses `RuntimeError`, and centrally log the rejection there. `futures_util` is already a transitive dependency of server SDKs (via e.g. `hyper` and `tower`), so adding it is a direct dependency is not worse. This helps with #2521. ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._
1 parent 6806f42 commit 3e02edc

File tree

6 files changed

+22
-7
lines changed

6 files changed

+22
-7
lines changed

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/ServerCargoDependency.kt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import software.amazon.smithy.rust.codegen.core.smithy.RuntimeConfig
1717
object ServerCargoDependency {
1818
val AsyncTrait: CargoDependency = CargoDependency("async-trait", CratesIo("0.1"))
1919
val FormUrlEncoded: CargoDependency = CargoDependency("form_urlencoded", CratesIo("1"))
20+
val FuturesUtil: CargoDependency = CargoDependency("futures-util", CratesIo("0.3"))
2021
val Mime: CargoDependency = CargoDependency("mime", CratesIo("0.3"))
2122
val Nom: CargoDependency = CargoDependency("nom", CratesIo("7"))
2223
val OnceCell: CargoDependency = CargoDependency("once_cell", CratesIo("1.13"))

codegen-server/src/main/kotlin/software/amazon/smithy/rust/codegen/server/smithy/protocols/ServerHttpBoundProtocolGenerator.kt

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -137,6 +137,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
137137
"Cow" to RuntimeType.Cow,
138138
"DateTime" to RuntimeType.dateTime(runtimeConfig),
139139
"FormUrlEncoded" to ServerCargoDependency.FormUrlEncoded.toType(),
140+
"FuturesUtil" to ServerCargoDependency.FuturesUtil.toType(),
140141
"HttpBody" to RuntimeType.HttpBody,
141142
"header_util" to RuntimeType.smithyHttp(runtimeConfig).resolve("header"),
142143
"Hyper" to RuntimeType.Hyper,
@@ -182,7 +183,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
182183
rustTemplate(
183184
"""
184185
if !#{SmithyHttpServer}::protocols::accept_header_classifier(request.headers(), ${contentType.dq()}) {
185-
return Err(#{RuntimeError}::NotAcceptable)
186+
return Err(#{RequestRejection}::NotAcceptable);
186187
}
187188
""",
188189
*codegenScope,
@@ -201,9 +202,7 @@ class ServerHttpBoundProtocolTraitImplGenerator(
201202
?.let { "Some(${it.dq()})" } ?: "None"
202203
rustTemplate(
203204
"""
204-
if #{SmithyHttpServer}::protocols::content_type_header_classifier(request.headers(), $expectedRequestContentType).is_err() {
205-
return Err(#{RuntimeError}::UnsupportedMediaType)
206-
}
205+
#{SmithyHttpServer}::protocols::content_type_header_classifier(request.headers(), $expectedRequestContentType)?;
207206
""",
208207
*codegenScope,
209208
)
@@ -213,9 +212,9 @@ class ServerHttpBoundProtocolTraitImplGenerator(
213212

214213
// Implement `from_request` trait for input types.
215214
val inputFuture = "${inputSymbol.name}Future"
215+
// TODO(https://github.com/awslabs/smithy-rs/issues/2238): Remove the `Pin<Box<dyn Future>>` and replace with thin wrapper around `Collect`.
216216
rustTemplate(
217217
"""
218-
// TODO(https://github.com/awslabs/smithy-rs/issues/2238): Remove the `Pin<Box<dyn Future>>` and replace with thin wrapper around `Collect`.
219218
#{PinProjectLite}::pin_project! {
220219
/// A [`Future`](std::future::Future) aggregating the body bytes of a [`Request`] and constructing the
221220
/// [`${inputSymbol.name}`](#{I}) using modelled bindings.
@@ -252,13 +251,17 @@ class ServerHttpBoundProtocolTraitImplGenerator(
252251
.await
253252
.map_err(Into::into)
254253
};
254+
use #{FuturesUtil}::future::TryFutureExt;
255+
let fut = fut.map_err(|e: #{RequestRejection}| {
256+
#{Tracing}::debug!(error = %e, "failed to deserialize request");
257+
#{RuntimeError}::from(e)
258+
});
255259
$inputFuture {
256260
inner: Box::pin(fut)
257261
}
258262
}
259263
}
260-
261-
""".trimIndent(),
264+
""",
262265
*codegenScope,
263266
"I" to inputSymbol,
264267
"Marker" to protocol.markerStruct(),

rust-runtime/aws-smithy-http-server/src/proto/aws_json/rejection.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ pub enum ResponseRejection {
1818
pub enum RequestRejection {
1919
#[error("error converting non-streaming body to bytes: {0}")]
2020
BufferHttpBodyBytes(crate::Error),
21+
#[error("request contains invalid value for `Accept` header")]
22+
NotAcceptable,
2123
#[error("expected `Content-Type` header not found: {0}")]
2224
MissingContentType(#[from] MissingContentTypeReason),
2325
#[error("error deserializing request HTTP body as JSON: {0}")]

rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/rejection.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -109,6 +109,11 @@ pub enum RequestRejection {
109109
#[error("error converting non-streaming body to bytes: {0}")]
110110
BufferHttpBodyBytes(crate::Error),
111111

112+
/// Used when the request contained an `Accept` header with a MIME type, and the server cannot
113+
/// return a response body adhering to that MIME type.
114+
#[error("request contains invalid value for `Accept` header")]
115+
NotAcceptable,
116+
112117
/// Used when checking the `Content-Type` header.
113118
#[error("expected `Content-Type` header not found: {0}")]
114119
MissingContentType(#[from] MissingContentTypeReason),

rust-runtime/aws-smithy-http-server/src/proto/rest_json_1/runtime_error.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ impl From<RequestRejection> for RuntimeError {
115115
match err {
116116
RequestRejection::MissingContentType(_reason) => Self::UnsupportedMediaType,
117117
RequestRejection::ConstraintViolation(reason) => Self::Validation(reason),
118+
RequestRejection::NotAcceptable => Self::NotAcceptable,
118119
_ => Self::Serialization(crate::Error::new(err)),
119120
}
120121
}

rust-runtime/aws-smithy-http-server/src/proto/rest_xml/rejection.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@ pub enum RequestRejection {
2828
#[error("error converting non-streaming body to bytes: {0}")]
2929
BufferHttpBodyBytes(crate::Error),
3030

31+
#[error("request contains invalid value for `Accept` header")]
32+
NotAcceptable,
33+
3134
#[error("expected `Content-Type` header not found: {0}")]
3235
MissingContentType(#[from] MissingContentTypeReason),
3336

0 commit comments

Comments
 (0)