Skip to content

Commit

Permalink
feat: Convert some of the validation storage errors into metrics (#810)
Browse files Browse the repository at this point in the history
* feat: Convert some of the validation storage errors into metrics

Closes #795

* f fmt, clippy

* f remove unused function, added hawk invalid_port

Co-authored-by: Donovan Preston <[email protected]>
Co-authored-by: Rachel Tublitz <[email protected]>
  • Loading branch information
3 people authored Sep 1, 2020
1 parent a79f840 commit 66221d8
Show file tree
Hide file tree
Showing 5 changed files with 131 additions and 28 deletions.
25 changes: 21 additions & 4 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ pub const RETRY_AFTER: u8 = 10;
pub struct ApiError {
inner: Context<ApiErrorKind>,
status: StatusCode,
metric_label: Option<String>,
}

/// Top-level ErrorKind.
Expand Down Expand Up @@ -130,8 +131,8 @@ impl ApiError {
_ => (),
},
_ => (),
}
true
};
self.metric_label.is_none()
}

pub fn on_response(&self, state: &ServerState) {
Expand All @@ -148,6 +149,7 @@ impl ApiError {
ref location,
name,
ref _tags,
ref _metric_label,
) => {
match description.as_ref() {
"over-quota" => return WeaveError::OverQuota,
Expand All @@ -162,7 +164,12 @@ impl ApiError {
}
WeaveError::UnknownError
}
ValidationErrorKind::FromValidationErrors(ref _err, ref location, ref _tags) => {
ValidationErrorKind::FromValidationErrors(
ref _err,
ref location,
ref _tags,
_metric_label,
) => {
if *location == RequestErrorLocation::Body {
WeaveError::InvalidWbo
} else {
Expand Down Expand Up @@ -225,7 +232,11 @@ impl From<Context<ApiErrorKind>> for ApiError {
ApiErrorKind::Validation(error) => error.status,
};

Self { inner, status }
Self {
inner,
status,
metric_label: None,
}
}
}

Expand Down Expand Up @@ -338,3 +349,9 @@ macro_rules! from_error {
from_error!(DbError, ApiError, ApiErrorKind::Db);
from_error!(HawkError, ApiError, ApiErrorKind::Hawk);
from_error!(ValidationError, ApiError, ApiErrorKind::Validation);

macro_rules! label {
($string:expr) => {
Some($string.to_string())
};
}
1 change: 1 addition & 0 deletions src/web/auth.rs
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ impl HawkPayload {
RequestErrorLocation::Header,
None,
tags,
label!("request.validate.hawk.invalid_port"),
)
})?
} else if ci.scheme() == "https" {
Expand Down
45 changes: 36 additions & 9 deletions src/web/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,13 +90,20 @@ impl ValidationError {
#[derive(Debug, Fail)]
pub enum ValidationErrorKind {
#[fail(display = "{}", _0)]
FromDetails(String, RequestErrorLocation, Option<String>, Option<Tags>),
FromDetails(
String,
RequestErrorLocation,
Option<String>,
Option<Tags>,
Option<String>,
),

#[fail(display = "{}", _0)]
FromValidationErrors(
#[cause] validator::ValidationErrors,
RequestErrorLocation,
Option<Tags>,
Option<String>,
),
}

Expand All @@ -119,18 +126,27 @@ impl From<Context<ValidationErrorKind>> for ValidationError {
fn from(inner: Context<ValidationErrorKind>) -> Self {
debug!("Validation Error: {:?}", inner.get_context());
let status = match inner.get_context() {
ValidationErrorKind::FromDetails(ref _description, ref location, Some(ref name), _)
if *location == RequestErrorLocation::Header =>
{
ValidationErrorKind::FromDetails(
ref _description,
ref location,
Some(ref name),
_,
_,
) if *location == RequestErrorLocation::Header => {
match name.to_ascii_lowercase().as_str() {
"accept" => StatusCode::NOT_ACCEPTABLE,
"content-type" => StatusCode::UNSUPPORTED_MEDIA_TYPE,
_ => StatusCode::BAD_REQUEST,
}
}
ValidationErrorKind::FromDetails(ref _description, ref location, Some(ref name), _)
if *location == RequestErrorLocation::Path
&& ["bso", "collection"].contains(&name.as_ref()) =>
ValidationErrorKind::FromDetails(
ref _description,
ref location,
Some(ref name),
_,
_,
) if *location == RequestErrorLocation::Path
&& ["bso", "collection"].contains(&name.as_ref()) =>
{
StatusCode::NOT_FOUND
}
Expand Down Expand Up @@ -185,7 +201,13 @@ impl Serialize for ValidationErrorKind {
let mut seq = serializer.serialize_seq(None)?;

match *self {
ValidationErrorKind::FromDetails(ref description, ref location, ref name, ref tags) => {
ValidationErrorKind::FromDetails(
ref description,
ref location,
ref name,
ref tags,
ref _metric_label,
) => {
seq.serialize_element(&SerializedValidationError {
description,
location,
Expand All @@ -195,7 +217,12 @@ impl Serialize for ValidationErrorKind {
})?;
}

ValidationErrorKind::FromValidationErrors(ref errors, ref location, ref tags) => {
ValidationErrorKind::FromValidationErrors(
ref errors,
ref location,
ref tags,
ref _metric_label,
) => {
for (field, field_errors) in errors.clone().field_errors().iter() {
for field_error in field_errors.iter() {
seq.serialize_element(&SerializedValidationError {
Expand Down
Loading

0 comments on commit 66221d8

Please sign in to comment.