Skip to content

Commit

Permalink
Merge pull request #207 from ajjahn/errors
Browse files Browse the repository at this point in the history
Severity trait & fix wildcard enum foot gun
  • Loading branch information
huntharo authored May 2, 2024
2 parents a492d02 + 2a810ee commit 7f82820
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 116 deletions.
93 changes: 31 additions & 62 deletions extension/src/lambda_request_error.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use crate::messages::ExitReason;
use crate::{messages::ExitReason, prelude::*};

#[derive(Debug, PartialEq)]
#[derive(Debug, PartialEq, Copy, Clone)]
pub enum LambdaRequestError {
RouterLambdaInvokeInvalid,
RouterUnreachable,
Expand Down Expand Up @@ -64,53 +64,22 @@ impl From<&LambdaRequestError> for ExitReason {

impl LambdaRequestError {
pub fn is_fatal(&self) -> bool {
use LambdaRequestError::*;

let fatal_errors = [
// Add other fatal errors here
AppConnectionUnreachable,
];

fatal_errors.contains(self)
}

pub fn worse(self, other: Self) -> Self {
use LambdaRequestError::*;

let ordered_reasons = [
RouterLambdaInvokeInvalid,
RouterUnreachable,
RouterConnectionError,
AppConnectionUnreachable,
AppConnectionError,
ChannelErrorOther,
];

for reason in ordered_reasons {
if self == reason || other == reason {
return reason;
}
}

self // If no match is found, return the current value
// Add other fatal errors to the match pattern. For example:
// matches!(self, Self::Variant1 | Self::Variant2)
matches!(self, Self::AppConnectionUnreachable)
}
}

// This function will cause a compile-time error if a new variant is added to the enum
// but not added to the match expression.
#[allow(dead_code)]
fn ensure_all_variants_handled(variant: Self) {
use LambdaRequestError::*;

match variant {
// HEY - If you add here you need to add to the `worse` function array above
RouterLambdaInvokeInvalid => {}
RouterUnreachable => {}
RouterConnectionError => {}
AppConnectionUnreachable => {}
AppConnectionError => {}
ChannelErrorOther => {}
impl Severity for LambdaRequestError {
fn severity(&self) -> usize {
match self {
Self::RouterLambdaInvokeInvalid => 0,
Self::RouterUnreachable => 1,
Self::RouterConnectionError => 2,
Self::AppConnectionUnreachable => 3,
Self::AppConnectionError => 4,
Self::ChannelErrorOther => 5,
}
// HEY - If you add here you need to add to the `worse` function array above
}
}

Expand Down Expand Up @@ -166,46 +135,46 @@ mod tests {

#[test]
fn test_worse() {
use LambdaRequestError::*;
use LambdaRequestError as E;

// Test that RouterUnreachable is considered worse than AppConnectionError
let error1 = RouterUnreachable;
let error2 = AppConnectionError;
assert_eq!(error1.worse(error2), RouterUnreachable);
let error1 = E::RouterUnreachable;
let error2 = E::AppConnectionError;
assert_eq!(error1.worse(error2), E::RouterUnreachable);

// Test that AppConnectionError is considered worse than ChannelErrorOther
let error1 = AppConnectionError;
let error2 = ChannelErrorOther;
assert_eq!(error1.worse(error2), AppConnectionError);
let error1 = E::AppConnectionError;
let error2 = E::ChannelErrorOther;
assert_eq!(error1.worse(error2), E::AppConnectionError);

// Test that when both errors are the same, that error is returned
let error1 = ChannelErrorOther;
let error2 = ChannelErrorOther;
assert_eq!(error1.worse(error2), ChannelErrorOther);
let error1 = E::ChannelErrorOther;
let error2 = E::ChannelErrorOther;
assert_eq!(error1.worse(error2), E::ChannelErrorOther);
}

#[test]
fn test_is_fatal() {
use LambdaRequestError::*;
use LambdaRequestError as E;

// Test that AppConnectionUnreachable is considered fatal
let error = AppConnectionUnreachable;
let error = E::AppConnectionUnreachable;
assert!(error.is_fatal());

// Test that RouterUnreachable is not considered fatal
let error = RouterUnreachable;
let error = E::RouterUnreachable;
assert!(!error.is_fatal());

// Test that RouterConnectionError is not considered fatal
let error = RouterConnectionError;
let error = E::RouterConnectionError;
assert!(!error.is_fatal());

// Test that AppConnectionError is not considered fatal
let error = AppConnectionError;
let error = E::AppConnectionError;
assert!(!error.is_fatal());

// Test that ChannelErrorOther is not considered fatal
let error = ChannelErrorOther;
let error = E::ChannelErrorOther;
assert!(!error.is_fatal());
}
}
70 changes: 17 additions & 53 deletions extension/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,60 +70,24 @@ pub enum ExitReason {
ChannelErrorOther,
}

impl ExitReason {
pub fn worse(self, other: Self) -> Self {
use ExitReason::*;

let ordered_reasons = [
RouterLambdaInvokeInvalid,
RouterUnreachable,
RouterConnectionError,
ChannelErrorOther,
AppConnectionError,
AppConnectionClosed,
RouterStatus5xx,
RouterStatus4xx,
RouterStatusOther,
RouterGoAway,
SelfLastActive,
SelfDeadline,
SelfStaleRequest,
SelfInitOnly,
];

for reason in ordered_reasons {
if self == reason || other == reason {
return reason;
}
}

self // If no match is found, return the current value
}

// This function will cause a compile-time error if a new variant is added to the enum
// but not added to the match expression.
#[allow(dead_code)]
fn ensure_all_variants_handled(variant: Self) {
use ExitReason::*;

match variant {
// HEY - If you add here you need to add to the `worse` function array above
RouterLambdaInvokeInvalid => {}
RouterUnreachable => {}
RouterConnectionError => {}
ChannelErrorOther => {}
AppConnectionError => {}
AppConnectionClosed => {}
RouterStatus5xx => {}
RouterStatus4xx => {}
RouterStatusOther => {}
RouterGoAway => {}
SelfLastActive => {}
SelfDeadline => {}
SelfStaleRequest => {}
SelfInitOnly => {}
impl Severity for ExitReason {
fn severity(&self) -> usize {
match self {
Self::RouterLambdaInvokeInvalid => 0,
Self::RouterUnreachable => 1,
Self::RouterConnectionError => 2,
Self::ChannelErrorOther => 3,
Self::AppConnectionError => 4,
Self::AppConnectionClosed => 5,
Self::RouterStatus5xx => 6,
Self::RouterStatus4xx => 7,
Self::RouterStatusOther => 8,
Self::RouterGoAway => 9,
Self::SelfLastActive => 10,
Self::SelfDeadline => 11,
Self::SelfStaleRequest => 12,
Self::SelfInitOnly => 13,
}
// HEY - If you add here you need to add to the `worse` function array above
}
}

Expand Down
13 changes: 13 additions & 0 deletions extension/src/prelude.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,3 +4,16 @@ pub use anyhow::{Context, Error, Result};
pub type LambdaId = Arc<str>;
pub type PoolId = Arc<str>;
pub type ChannelId = Arc<str>;

pub trait Severity: Sized {
/// Lower values represent higher severity with zero being the highest (akin to SEV0, SEV1, etc).
fn severity(&self) -> usize;

/// Compares two [`Severity`]s, returning the highest.
fn worse(self, other: Self) -> Self {
if self.severity() > other.severity() {
return other;
}
self
}
}
5 changes: 4 additions & 1 deletion extension/tests/messages.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
#[cfg(test)]
mod tests {
use chrono::{DateTime, Utc};
use extension::messages::{ExitReason, ValidationError, WaiterRequest, WaiterResponse};
use extension::{
messages::{ExitReason, ValidationError, WaiterRequest, WaiterResponse},
prelude::*,
};
use hyper::Uri;
use serde_json::json;

Expand Down

0 comments on commit 7f82820

Please sign in to comment.