diff --git a/autoendpoint/src/error.rs b/autoendpoint/src/error.rs index b4cc27f6..16835273 100644 --- a/autoendpoint/src/error.rs +++ b/autoendpoint/src/error.rs @@ -18,8 +18,8 @@ use thiserror::Error; /// Common `Result` type. pub type ApiResult = Result; -/// How long the client should wait before retrying a conflicting write. -pub const RETRY_AFTER: u8 = 10; +/// A link for more info on the returned error +const ERROR_URL: &str = "http://autopush.readthedocs.io/en/latest/http.html#error-codes"; /// The main error type. #[derive(Debug)] @@ -74,6 +74,9 @@ pub enum ApiErrorKind { #[error("Invalid token")] InvalidToken, + #[error("UAID not found")] + NoUser, + #[error("No such subscription")] NoSubscription, @@ -103,7 +106,7 @@ impl ApiErrorKind { | ApiErrorKind::TokenHashValidation(_) | ApiErrorKind::Uuid(_) => StatusCode::BAD_REQUEST, - ApiErrorKind::NoSubscription => StatusCode::GONE, + ApiErrorKind::NoUser | ApiErrorKind::NoSubscription => StatusCode::GONE, ApiErrorKind::VapidError(_) | ApiErrorKind::Jwt(_) => StatusCode::UNAUTHORIZED, @@ -117,6 +120,23 @@ impl ApiErrorKind { | ApiErrorKind::Internal(_) => StatusCode::INTERNAL_SERVER_ERROR, } } + + /// Get the associated error number + pub fn errno(&self) -> Option { + match self { + ApiErrorKind::InvalidEncryption(_) => Some(110), + ApiErrorKind::VapidError(_) + | ApiErrorKind::TokenHashValidation(_) + | ApiErrorKind::Jwt(_) => Some(109), + ApiErrorKind::InvalidToken => Some(102), + ApiErrorKind::NoUser => Some(103), + ApiErrorKind::NoSubscription => Some(106), + ApiErrorKind::PayloadError(PayloadError::Overflow) + | ApiErrorKind::PayloadTooLarge(_) => Some(104), + ApiErrorKind::Internal(_) => Some(999), + _ => None, + } + } } // Print out the error and backtrace, including source errors @@ -174,40 +194,23 @@ impl From for HttpResponse { impl ResponseError for ApiError { fn error_response(&self) -> HttpResponse { - // To return a descriptive error response, this would work. We do not - // unfortunately do that so that we can retain Sync 1.1 backwards compatibility - // as the Python one does. - // HttpResponse::build(self.status).json(self) - // - // So instead we translate our error to a backwards compatible one - HttpResponse::build(self.kind.status()) - .header("Retry-After", RETRY_AFTER.to_string()) - .finish() + HttpResponse::build(self.kind.status()).json(self) } } -// TODO: Use the same schema as documented here? -// https://autopush.readthedocs.io/en/latest/http.html#response impl Serialize for ApiError { fn serialize(&self, serializer: S) -> Result where S: Serializer, { let status = self.kind.status(); - let size = if status == StatusCode::UNAUTHORIZED { - 2 - } else { - 3 - }; - - let mut map = serializer.serialize_map(Some(size))?; - map.serialize_entry("status", &status.as_u16())?; - map.serialize_entry("reason", status.canonical_reason().unwrap_or(""))?; - - if status != StatusCode::UNAUTHORIZED { - map.serialize_entry("errors", &self.kind.to_string())?; - } + let mut map = serializer.serialize_map(Some(5))?; + map.serialize_entry("code", &status.as_u16())?; + map.serialize_entry("errno", &self.kind.errno())?; + map.serialize_entry("error", &status.canonical_reason())?; + map.serialize_entry("message", &self.kind.to_string())?; + map.serialize_entry("more_info", ERROR_URL)?; map.end() } } diff --git a/autoendpoint/src/server/extractors/subscription.rs b/autoendpoint/src/server/extractors/subscription.rs index 371b0444..77520b0a 100644 --- a/autoendpoint/src/server/extractors/subscription.rs +++ b/autoendpoint/src/server/extractors/subscription.rs @@ -63,7 +63,8 @@ impl FromRequest for Subscription { .get_user(&uaid) .compat() .await - .map_err(ApiErrorKind::Database)?; + .map_err(ApiErrorKind::Database)? + .ok_or(ApiErrorKind::NoUser)?; validate_user(&user, &channel_id, &state).await?; // Validate the VAPID JWT token and record the version diff --git a/autopush-common/src/db/mod.rs b/autopush-common/src/db/mod.rs index 22b411dd..291d508d 100644 --- a/autopush-common/src/db/mod.rs +++ b/autopush-common/src/db/mod.rs @@ -443,17 +443,17 @@ impl DynamoStorage { }) } - pub fn get_user(&self, uaid: &Uuid) -> impl Future { + pub fn get_user(&self, uaid: &Uuid) -> impl Future, Error = Error> { let ddb = self.ddb.clone(); let result = commands::get_uaid(ddb, uaid, &self.router_table_name).and_then(|result| { future::result( result .item - .ok_or_else(|| "No user record found".into()) - .and_then(|item| { + .map(|item| { let user = serde_dynamodb::from_hashmap(item); user.chain_err(|| "Error deserializing") - }), + }) + .transpose(), ) }); Box::new(result) diff --git a/autopush/src/client.rs b/autopush/src/client.rs index 1483eaec..8e15e209 100644 --- a/autopush/src/client.rs +++ b/autopush/src/client.rs @@ -693,6 +693,11 @@ fn save_and_notify_undelivered_messages( srv2.ddb.get_user(&uaid) }) .and_then(move |user| { + let user = match user.ok_or_else(|| "No user record found".into()) { + Ok(user) => user, + Err(e) => return future::err(e), + }; + // Return an err to stop processing if the user hasn't reconnected yet, otherwise // attempt to construct a client to make the request if user.connected_at == connected_at {