From 17c09e6a2cf0f063fff036cdf2cc4a9557d2cdcb Mon Sep 17 00:00:00 2001
From: Benoit Ranque <ranque.benoit@gmail.com>
Date: Tue, 2 Jul 2024 02:18:16 -0400
Subject: [PATCH] Remove threadsafe requirement for errors

---
 crates/sdk/src/connector.rs     | 108 ++++++++++++++------------------
 crates/sdk/src/default_main.rs  |  17 +++--
 crates/sdk/src/json_response.rs |   4 +-
 3 files changed, 56 insertions(+), 73 deletions(-)

diff --git a/crates/sdk/src/connector.rs b/crates/sdk/src/connector.rs
index 676353c..59f4764 100644
--- a/crates/sdk/src/connector.rs
+++ b/crates/sdk/src/connector.rs
@@ -123,12 +123,12 @@ pub enum InitializationError {
 #[derive(Debug, Error)]
 pub enum FetchMetricsError {
     #[error("error fetching metrics: {0}")]
-    Other(Box<dyn Error + Send + Sync>, serde_json::Value),
+    Other(Box<dyn Error>, serde_json::Value),
 }
 
 impl FetchMetricsError {
-    pub fn new(err: Box<dyn Error + Send + Sync>) -> Self {
-        Self::Other(err, serde_json::Value::Null)
+    pub fn new<E: Into<Box<dyn Error>>>(err: E) -> Self {
+        Self::Other(err.into(), serde_json::Value::Null)
     }
     #[must_use]
     pub fn with_details(self, details: serde_json::Value) -> Self {
@@ -138,12 +138,6 @@ impl FetchMetricsError {
     }
 }
 
-impl From<Box<dyn Error + Send + Sync>> for FetchMetricsError {
-    fn from(value: Box<dyn Error + Send + Sync>) -> Self {
-        Self::new(value)
-    }
-}
-
 impl IntoResponse for FetchMetricsError {
     fn into_response(self) -> Response {
         match self {
@@ -165,12 +159,12 @@ impl IntoResponse for FetchMetricsError {
 #[derive(Debug, Error)]
 pub enum HealthError {
     #[error("error checking health status: {0}")]
-    Other(Box<dyn Error + Send + Sync>, serde_json::Value),
+    Other(Box<dyn Error>, serde_json::Value),
 }
 
 impl HealthError {
-    pub fn new(err: Box<dyn Error + Send + Sync>) -> Self {
-        Self::Other(err, serde_json::Value::Null)
+    pub fn new<E: Into<Box<dyn Error>>>(err: E) -> Self {
+        Self::Other(err.into(), serde_json::Value::Null)
     }
     #[must_use]
     pub fn with_details(self, details: serde_json::Value) -> Self {
@@ -180,12 +174,6 @@ impl HealthError {
     }
 }
 
-impl From<Box<dyn Error + Send + Sync>> for HealthError {
-    fn from(value: Box<dyn Error + Send + Sync>) -> Self {
-        Self::new(value)
-    }
-}
-
 impl IntoResponse for HealthError {
     fn into_response(self) -> Response {
         match self {
@@ -207,12 +195,12 @@ impl IntoResponse for HealthError {
 #[derive(Debug, Error)]
 pub enum SchemaError {
     #[error("error retrieving the schema: {0}")]
-    Other(Box<dyn Error + Send + Sync>, serde_json::Value),
+    Other(Box<dyn Error>, serde_json::Value),
 }
 
 impl SchemaError {
-    pub fn new(err: Box<dyn Error + Send + Sync>) -> Self {
-        Self::Other(err, serde_json::Value::Null)
+    pub fn new<E: Into<Box<dyn Error>>>(err: E) -> Self {
+        Self::Other(err.into(), serde_json::Value::Null)
     }
     #[must_use]
     pub fn with_details(self, details: serde_json::Value) -> Self {
@@ -222,8 +210,8 @@ impl SchemaError {
     }
 }
 
-impl From<Box<dyn Error + Send + Sync>> for SchemaError {
-    fn from(value: Box<dyn Error + Send + Sync>) -> Self {
+impl From<Box<dyn Error>> for SchemaError {
+    fn from(value: Box<dyn Error>) -> Self {
         Self::new(value)
     }
 }
@@ -264,28 +252,28 @@ pub enum QueryError {
     #[error("unsupported operation: {}", .0.message)]
     UnsupportedOperation(models::ErrorResponse),
     #[error("error executing query: {0}")]
-    Other(Box<dyn Error + Send + Sync>, serde_json::Value),
+    Other(Box<dyn Error>, serde_json::Value),
 }
 
 impl QueryError {
-    pub fn new(err: Box<dyn Error + Send + Sync>) -> Self {
-        Self::Other(err, serde_json::Value::Null)
+    pub fn new<E: Into<Box<dyn Error>>>(err: E) -> Self {
+        Self::Other(err.into(), serde_json::Value::Null)
     }
-    pub fn new_invalid_request<T: Into<String>>(message: T) -> Self {
+    pub fn new_invalid_request<T: ToString>(message: &T) -> Self {
         Self::InvalidRequest(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_unprocessable_content<T: Into<String>>(message: T) -> Self {
+    pub fn new_unprocessable_content<T: ToString>(message: &T) -> Self {
         Self::UnprocessableContent(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_unsupported_operation<T: Into<String>>(message: T) -> Self {
+    pub fn new_unsupported_operation<T: ToString>(message: &T) -> Self {
         Self::UnsupportedOperation(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
@@ -306,8 +294,8 @@ impl QueryError {
     }
 }
 
-impl From<Box<dyn Error + Send + Sync>> for QueryError {
-    fn from(value: Box<dyn Error + Send + Sync>) -> Self {
+impl From<Box<dyn Error>> for QueryError {
+    fn from(value: Box<dyn Error>) -> Self {
         Self::new(value)
     }
 }
@@ -351,28 +339,28 @@ pub enum ExplainError {
     #[error("unsupported operation: {}", .0.message)]
     UnsupportedOperation(models::ErrorResponse),
     #[error("explain error: {0}")]
-    Other(Box<dyn Error + Send + Sync>, serde_json::Value),
+    Other(Box<dyn Error>, serde_json::Value),
 }
 
 impl ExplainError {
-    pub fn new(err: Box<dyn Error + Send + Sync>) -> Self {
-        Self::Other(err, serde_json::Value::Null)
+    pub fn new<E: Into<Box<dyn Error>>>(err: E) -> Self {
+        Self::Other(err.into(), serde_json::Value::Null)
     }
-    pub fn new_invalid_request<T: Into<String>>(message: T) -> Self {
+    pub fn new_invalid_request<T: ToString>(message: &T) -> Self {
         Self::InvalidRequest(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_unprocessable_content<T: Into<String>>(message: T) -> Self {
+    pub fn new_unprocessable_content<T: ToString>(message: &T) -> Self {
         Self::UnprocessableContent(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_unsupported_operation<T: Into<String>>(message: T) -> Self {
+    pub fn new_unsupported_operation<T: ToString>(message: &T) -> Self {
         Self::UnsupportedOperation(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
@@ -393,8 +381,8 @@ impl ExplainError {
     }
 }
 
-impl From<Box<dyn Error + Send + Sync>> for ExplainError {
-    fn from(value: Box<dyn Error + Send + Sync>) -> Self {
+impl From<Box<dyn Error>> for ExplainError {
+    fn from(value: Box<dyn Error>) -> Self {
         Self::new(value)
     }
 }
@@ -446,40 +434,40 @@ pub enum MutationError {
     #[error("mutation violates constraint: {}", .0.message)]
     ConstraintNotMet(models::ErrorResponse),
     #[error("error executing mutation: {0}")]
-    Other(Box<dyn Error + Send + Sync>, serde_json::Value),
+    Other(Box<dyn Error>, serde_json::Value),
 }
 
 impl MutationError {
-    pub fn new(err: Box<dyn Error + Send + Sync>) -> Self {
-        Self::Other(err, serde_json::Value::Null)
+    pub fn new<E: Into<Box<dyn Error>>>(err: E) -> Self {
+        Self::Other(err.into(), serde_json::Value::Null)
     }
-    pub fn new_invalid_request<T: Into<String>>(message: T) -> Self {
+    pub fn new_invalid_request<T: ToString>(message: &T) -> Self {
         Self::InvalidRequest(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_unprocessable_content<T: Into<String>>(message: T) -> Self {
+    pub fn new_unprocessable_content<T: ToString>(message: &T) -> Self {
         Self::UnprocessableContent(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_unsupported_operation<T: Into<String>>(message: T) -> Self {
+    pub fn new_unsupported_operation<T: ToString>(message: &T) -> Self {
         Self::UnsupportedOperation(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_conflict<T: Into<String>>(message: T) -> Self {
+    pub fn new_conflict<T: ToString>(message: &T) -> Self {
         Self::Conflict(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
-    pub fn new_constraint_not_met<T: Into<String>>(message: T) -> Self {
+    pub fn new_constraint_not_met<T: ToString>(message: &T) -> Self {
         Self::ConstraintNotMet(models::ErrorResponse {
-            message: message.into(),
+            message: message.to_string(),
             details: serde_json::Value::Null,
         })
     }
@@ -506,8 +494,8 @@ impl MutationError {
     }
 }
 
-impl From<Box<dyn Error + Send + Sync>> for MutationError {
-    fn from(value: Box<dyn Error + Send + Sync>) -> Self {
+impl From<Box<dyn Error>> for MutationError {
+    fn from(value: Box<dyn Error>) -> Self {
         Self::new(value)
     }
 }
diff --git a/crates/sdk/src/default_main.rs b/crates/sdk/src/default_main.rs
index 729a8a5..7d10adc 100644
--- a/crates/sdk/src/default_main.rs
+++ b/crates/sdk/src/default_main.rs
@@ -384,7 +384,7 @@ async fn get_metrics<C: Connector>(
 
     encoder
         .encode_to_string(metric_families)
-        .map_err(|_| FetchMetricsError::new("Unable to encode metrics".into()))
+        .map_err(|_| FetchMetricsError::new("Unable to encode metrics"))
 }
 
 async fn get_capabilities<C: Connector>() -> JsonResponse<CapabilitiesResponse> {
@@ -453,15 +453,15 @@ mod ndc_test_commands {
         ) -> Result<ndc_models::CapabilitiesResponse, ndc_test::error::Error> {
             C::get_capabilities()
                 .await
-                .into_value::<Box<dyn std::error::Error + Send + Sync>>()
-                .map_err(|err| ndc_test::error::Error::OtherError(err))
+                .into_value::<Box<dyn std::error::Error>>()
+                .map_err(ndc_test::error::Error::OtherError)
         }
 
         async fn get_schema(&self) -> Result<ndc_models::SchemaResponse, ndc_test::error::Error> {
             match C::get_schema(&self.configuration).await {
                 Ok(response) => response
-                    .into_value::<Box<dyn std::error::Error + Send + Sync>>()
-                    .map_err(|err| ndc_test::error::Error::OtherError(err)),
+                    .into_value::<Box<dyn std::error::Error>>()
+                    .map_err(ndc_test::error::Error::OtherError),
                 Err(err) => Err(ndc_test::error::Error::OtherError(err.into())),
             }
         }
@@ -487,13 +487,10 @@ mod ndc_test_commands {
             &self,
             request: ndc_models::MutationRequest,
         ) -> Result<ndc_models::MutationResponse, ndc_test::error::Error> {
-            match C::mutation(&self.configuration, &self.state, request)
+            Ok(C::mutation(&self.configuration, &self.state, request)
                 .await
                 .and_then(JsonResponse::into_value)
-            {
-                Ok(response) => Ok(response),
-                Err(err) => Err(ndc_test::error::Error::OtherError(err.into())),
-            }
+                .map_err(|err| ndc_test::error::Error::OtherError(err.into()))?)
         }
     }
 
diff --git a/crates/sdk/src/json_response.rs b/crates/sdk/src/json_response.rs
index 85be6e8..4789177 100644
--- a/crates/sdk/src/json_response.rs
+++ b/crates/sdk/src/json_response.rs
@@ -27,9 +27,7 @@ impl<A: (for<'de> serde::Deserialize<'de>)> JsonResponse<A> {
     ///
     /// This is only intended for testing and compatibility. If it lives on a
     /// critical path, we recommend you avoid it.
-    pub(crate) fn into_value<E: From<Box<dyn std::error::Error + Send + Sync>>>(
-        self,
-    ) -> Result<A, E> {
+    pub(crate) fn into_value<E: From<Box<dyn std::error::Error>>>(self) -> Result<A, E> {
         match self {
             Self::Value(value) => Ok(value),
             Self::Serialized(bytes) => {