severity trait, wildcard enum footgun

Author: ajjahn

extension/src/lambda_request_error.rs

@@ -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,
@@ -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
   }
 }
 
@@ -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());
   }
 }

extension/src/messages.rs

@@ -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
   }
 }
 

extension/src/prelude.rs

@@ -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
+  }
+}

extension/tests/messages.rs

@@ -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;