Skip to content

Commit

Permalink
Feature/408 request timeout (#104)
Browse files Browse the repository at this point in the history
* Return 408 Request Timeout on handler timeout instead of the socket disconnecting

* Attempt to produce a HTTP response for any SaphirError.

* Log SaphirError even when failing to produce a HTTP response.

* Cargo fmt

* Cargo clippy

* Remove State alteration mistakenly commited

* Lighter context clone without state for error

* Remove runtime branching of timeout, resolve it at server start instead

* Cargo fmt + clippy

* Properly select timeout handling on server start and not on each connection.
  • Loading branch information
Samuel-B-D authored Sep 14, 2020
1 parent 1b0c278 commit 31e2258
Show file tree
Hide file tree
Showing 5 changed files with 194 additions and 95 deletions.
2 changes: 1 addition & 1 deletion saphir/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "saphir"
version = "2.7.3"
version = "2.7.4"
edition = "2018"
authors = ["Richer Archambault <[email protected]>"]
description = "Fully async-await http server framework"
Expand Down
175 changes: 97 additions & 78 deletions saphir/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@ pub enum SaphirError {
MissingParameter(String, bool),
///
InvalidParameter(String, bool),
///
RequestTimeout,
}

impl Debug for SaphirError {
Expand All @@ -77,6 +79,7 @@ impl Debug for SaphirError {
SaphirError::SerdeUrlSer(d) => std::fmt::Debug::fmt(d, f),
SaphirError::MissingParameter(d, _) => std::fmt::Debug::fmt(d, f),
SaphirError::InvalidParameter(d, _) => std::fmt::Debug::fmt(d, f),
SaphirError::RequestTimeout => f.write_str("RequestTimeout"),
}
}
}
Expand All @@ -85,70 +88,30 @@ impl SaphirError {
pub fn responder<T: Responder + Send + Sync + 'static>(e: T) -> Self {
SaphirError::Responder(Box::new(Some(e)))
}
}

#[cfg(feature = "json")]
impl From<serde_json::error::Error> for SaphirError {
fn from(e: serde_json::error::Error) -> Self {
SaphirError::SerdeJson(e)
}
}

#[cfg(feature = "form")]
impl From<serde_urlencoded::de::Error> for SaphirError {
fn from(e: serde_urlencoded::de::Error) -> Self {
SaphirError::SerdeUrlDe(e)
}
}

#[cfg(feature = "form")]
impl From<serde_urlencoded::ser::Error> for SaphirError {
fn from(e: serde_urlencoded::ser::Error) -> Self {
SaphirError::SerdeUrlSer(e)
}
}

impl From<HttpCrateError> for SaphirError {
fn from(e: HttpCrateError) -> Self {
SaphirError::Internal(InternalError::Http(e))
}
}

impl From<InvalidHeaderValue> for SaphirError {
fn from(e: InvalidHeaderValue) -> Self {
SaphirError::Internal(InternalError::Http(HttpCrateError::from(e)))
}
}

impl From<HyperError> for SaphirError {
fn from(e: HyperError) -> Self {
SaphirError::Internal(InternalError::Hyper(e))
}
}

impl From<IoError> for SaphirError {
fn from(e: IoError) -> Self {
SaphirError::Io(e)
}
}

impl From<ToStrError> for SaphirError {
fn from(e: ToStrError) -> Self {
SaphirError::Internal(InternalError::ToStr(e))
}
}

impl Display for SaphirError {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> {
f.write_str("saphirError")
pub(crate) fn response_builder(self, builder: Builder, ctx: &HttpContext) -> Builder {
match self {
SaphirError::Internal(_) => builder.status(500),
SaphirError::Io(_) => builder.status(500),
SaphirError::BodyAlreadyTaken => builder.status(500),
SaphirError::Custom(_) => builder.status(500),
SaphirError::Other(_) => builder.status(500),
#[cfg(feature = "json")]
SaphirError::SerdeJson(_) => builder.status(400),
#[cfg(feature = "form")]
SaphirError::SerdeUrlDe(_) => builder.status(400),
#[cfg(feature = "form")]
SaphirError::SerdeUrlSer(_) => builder.status(400),
SaphirError::MissingParameter(..) => builder.status(400),
SaphirError::InvalidParameter(..) => builder.status(400),
SaphirError::RequestMovedBeforeHandler => builder.status(500),
SaphirError::ResponseMoved => builder.status(500),
SaphirError::Responder(mut r) => r.dyn_respond(builder, ctx),
SaphirError::RequestTimeout => builder.status(408),
}
}
}

impl StdError for SaphirError {}

impl Responder for SaphirError {
#[allow(unused_variables)]
fn respond_with_builder(self, builder: Builder, ctx: &HttpContext) -> Builder {
pub(crate) fn log(&self, ctx: &HttpContext) {
let op_id = {
#[cfg(not(feature = "operation"))]
{
Expand All @@ -164,69 +127,125 @@ impl Responder for SaphirError {
match self {
SaphirError::Internal(e) => {
warn!("{}Saphir encountered an internal error that was returned as a responder: {:?}", op_id, e);
builder.status(500)
}
SaphirError::Io(e) => {
warn!("{}Saphir encountered an Io error that was returned as a responder: {:?}", op_id, e);
builder.status(500)
}
SaphirError::BodyAlreadyTaken => {
warn!("{}A controller handler attempted to take the request body more thant one time", op_id);
builder.status(500)
}
SaphirError::Custom(e) => {
warn!("{}A custom error was returned as a responder: {:?}", op_id, e);
builder.status(500)
}
SaphirError::Other(e) => {
warn!("{}Saphir encountered an Unknown error that was returned as a responder: {:?}", op_id, e);
builder.status(500)
}
#[cfg(feature = "json")]
SaphirError::SerdeJson(e) => {
debug!("{}Unable to de/serialize json type: {:?}", op_id, e);
builder.status(400)
}
#[cfg(feature = "form")]
SaphirError::SerdeUrlDe(e) => {
debug!("{}Unable to deserialize form type: {:?}", op_id, e);
builder.status(400)
}
#[cfg(feature = "form")]
SaphirError::SerdeUrlSer(e) => {
debug!("{}Unable to serialize form type: {:?}", op_id, e);
builder.status(400)
}
SaphirError::MissingParameter(name, is_query) => {
if is_query {
if *is_query {
debug!("{}Missing query parameter {}", op_id, name);
} else {
debug!("{}Missing path parameter {}", op_id, name);
}

builder.status(400)
}
SaphirError::InvalidParameter(name, is_query) => {
if is_query {
if *is_query {
debug!("{}Unable to parse query parameter {}", op_id, name);
} else {
debug!("{}Unable to parse path parameter {}", op_id, name);
}

builder.status(400)
}
SaphirError::RequestMovedBeforeHandler => {
warn!(
"{}A request was moved out of its context by a middleware, but the middleware did not stop request processing",
op_id
);
builder.status(500)
}
SaphirError::ResponseMoved => {
warn!("{}A response was moved before being sent to the client", op_id);
builder.status(500)
}
SaphirError::Responder(mut r) => r.dyn_respond(builder, ctx),
SaphirError::RequestTimeout => {
warn!("{}Request timed out", op_id);
}
SaphirError::Responder(_) => {}
}
}
}

#[cfg(feature = "json")]
impl From<serde_json::error::Error> for SaphirError {
fn from(e: serde_json::error::Error) -> Self {
SaphirError::SerdeJson(e)
}
}

#[cfg(feature = "form")]
impl From<serde_urlencoded::de::Error> for SaphirError {
fn from(e: serde_urlencoded::de::Error) -> Self {
SaphirError::SerdeUrlDe(e)
}
}

#[cfg(feature = "form")]
impl From<serde_urlencoded::ser::Error> for SaphirError {
fn from(e: serde_urlencoded::ser::Error) -> Self {
SaphirError::SerdeUrlSer(e)
}
}

impl From<HttpCrateError> for SaphirError {
fn from(e: HttpCrateError) -> Self {
SaphirError::Internal(InternalError::Http(e))
}
}

impl From<InvalidHeaderValue> for SaphirError {
fn from(e: InvalidHeaderValue) -> Self {
SaphirError::Internal(InternalError::Http(HttpCrateError::from(e)))
}
}

impl From<HyperError> for SaphirError {
fn from(e: HyperError) -> Self {
SaphirError::Internal(InternalError::Hyper(e))
}
}

impl From<IoError> for SaphirError {
fn from(e: IoError) -> Self {
SaphirError::Io(e)
}
}

impl From<ToStrError> for SaphirError {
fn from(e: ToStrError) -> Self {
SaphirError::Internal(InternalError::ToStr(e))
}
}

impl Display for SaphirError {
fn fmt(&self, f: &mut Formatter<'_>) -> Result<(), FmtError> {
f.write_str("saphirError")
}
}

impl StdError for SaphirError {}

impl Responder for SaphirError {
#[allow(unused_variables)]
fn respond_with_builder(self, builder: Builder, ctx: &HttpContext) -> Builder {
self.log(ctx);
self.response_builder(builder, ctx)
}
}
10 changes: 10 additions & 0 deletions saphir/src/http_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,16 @@ impl HttpContext {
}
}

pub fn clone_with_empty_state(&self) -> Self {
HttpContext {
state: State::Empty,
router: self.router.clone(),
metadata: self.metadata.clone(),
#[cfg(feature = "operation")]
operation_id: self.operation_id,
}
}

/// Explicitly set the inner state to `Before` with the given response
pub fn before(&mut self, request: Request) {
self.state = State::Before(Box::new(request))
Expand Down
2 changes: 1 addition & 1 deletion saphir/src/router.rs
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ impl Router {
}
}

pub fn resolve_metadata(&self, req: &mut Request<Body>) -> HandlerMetadata {
pub fn resolve_metadata(&self, req: &mut Request) -> HandlerMetadata {
let mut method_not_allowed = false;

for endpoint_resolver in &self.inner.resolvers {
Expand Down
Loading

0 comments on commit 31e2258

Please sign in to comment.