Support the Accept-Conjure-Error-Parameter-Format header

witchcraft-server-ete/build.rs

@@ -21,6 +21,7 @@ fn main() {
     println!("cargo:rerun-if-changed={input}");
     conjure_codegen::Config::new()
         .strip_prefix("com.palantir.witchcraft.server.test".to_string())
+        .use_legacy_error_serialization(true)
         .generate_files(input, output)
         .unwrap();
 }

witchcraft-server-ete/src/async_handler.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 use crate::conjure::endpoints::AsyncTestService;
+use crate::conjure::errors::ComplexParameterError;
 use conjure_error::{Error, InvalidArgument};
 use conjure_http::server::AsyncWriteBody;
 use http::{HeaderMap, HeaderValue};
@@ -81,6 +82,16 @@ impl AsyncTestService<RequestBody, ResponseWriter> for TestResource {
 
         Ok(IoAfterEofBody)
     }
+
+    async fn error_response(&self) -> Result<(), Error> {
+        Err(Error::service_safe(
+            "blammo",
+            ComplexParameterError::builder()
+                .boolean(true)
+                .list(vec![1, 2, 3])
+                .build(),
+        ))
+    }
 }
 
 pub struct SlowBodyBody(Duration);

witchcraft-server-ete/src/handler.rs

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 use crate::conjure::endpoints::TestService;
+use crate::conjure::errors::ComplexParameterError;
 use conjure_error::{Error, InvalidArgument};
 use conjure_http::server::WriteBody;
 use http::{HeaderMap, HeaderValue};
@@ -78,6 +79,16 @@ impl TestService<RequestBody, ResponseWriter> for TestResource {
 
         Ok(IoAfterEofBody)
     }
+
+    fn error_response(&self) -> Result<(), Error> {
+        Err(Error::service_safe(
+            "blammo",
+            ComplexParameterError::builder()
+                .boolean(true)
+                .list(vec![1, 2, 3])
+                .build(),
+        ))
+    }
 }
 
 pub struct SlowBodyBody(Duration);

witchcraft-server-ete/test-ir.json

@@ -1,6 +1,32 @@
 {
   "version" : 1,
-  "errors" : [ ],
+  "errors" : [ {
+    "errorName" : {
+      "name" : "ComplexParameterError",
+      "package" : "com.palantir.witchcraft.server.test"
+    },
+    "namespace" : "Test",
+    "code" : "CUSTOM_CLIENT",
+    "safeArgs" : [ {
+      "fieldName" : "boolean",
+      "type" : {
+        "type" : "primitive",
+        "primitive" : "BOOLEAN"
+      }
+    }, {
+      "fieldName" : "list",
+      "type" : {
+        "type" : "list",
+        "list" : {
+          "itemType" : {
+            "type" : "primitive",
+            "primitive" : "INTEGER"
+          }
+        }
+      }
+    } ],
+    "unsafeArgs" : [ ]
+  } ],
   "types" : [ ],
   "services" : [ {
     "serviceName" : {
@@ -186,6 +212,13 @@
       },
       "markers" : [ ],
       "tags" : [ ]
+    }, {
+      "endpointName" : "errorResponse",
+      "httpMethod" : "GET",
+      "httpPath" : "/test/errorResponse",
+      "args" : [ ],
+      "markers" : [ ],
+      "tags" : [ ]
     } ]
   } ],
   "extensions" : { }

witchcraft-server-ete/test.yml

@@ -1,3 +1,13 @@
+types:
+  definitions:
+    default-package: com.palantir.witchcraft.server.test
+    errors:
+      ComplexParameterError:
+        namespace: Test
+        code: CUSTOM_CLIENT
+        safe-args:
+          boolean: boolean
+          list: list<integer>
 services:
   TestService:
     name: Test Service
@@ -55,3 +65,5 @@ services:
         args:
           body: binary
         returns: binary
+      errorResponse:
+        http: GET /errorResponse

witchcraft-server-ete/tests/ete/main.rs

@@ -12,12 +12,15 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 use bytes::Bytes;
+use conjure_error::SerializableError;
 use conjure_object::Any;
+use conjure_serde::json;
 use http::{HeaderMap, HeaderValue};
 use http_body_util::{BodyExt, Empty, Full};
 use hyper::body::{Body, Frame};
 use hyper::{Request, StatusCode};
 use server::Server;
+use std::collections::BTreeMap;
 use std::pin::Pin;
 use std::str;
 use std::task::{Context, Poll};
@@ -499,3 +502,52 @@ async fn management_port() {
         })
         .await;
 }
+
+#[tokio::test]
+async fn error_parameters() {
+    Server::builder()
+        .with(|server| async {
+            let request = Request::builder()
+                .uri("/witchcraft-ete/api/test/errorResponse")
+                .body(Empty::<Bytes>::new())
+                .unwrap();
+            let response = server
+                .client()
+                .await
+                .unwrap()
+                .send_request(request)
+                .await
+                .unwrap();
+
+            let body = response.into_body().collect().await.unwrap().to_bytes();
+            let error = json::client_from_slice::<SerializableError>(&body).unwrap();
+
+            let expected = BTreeMap::from([("boolean".to_string(), Any::new("true").unwrap())]);
+            assert_eq!(*error.parameters(), expected);
+
+            let request = Request::builder()
+                .uri("/witchcraft-ete/api/test/errorResponse")
+                .header("Accept-Conjure-Error-Parameter-Format", "JSON")
+                .body(Empty::<Bytes>::new())
+                .unwrap();
+            let response = server
+                .client()
+                .await
+                .unwrap()
+                .send_request(request)
+                .await
+                .unwrap();
+
+            let body = response.into_body().collect().await.unwrap().to_bytes();
+            let error = json::client_from_slice::<SerializableError>(&body).unwrap();
+
+            let expected = BTreeMap::from([
+                ("boolean".to_string(), Any::new(true).unwrap()),
+                ("list".to_string(), Any::new(vec![1u64, 2, 3]).unwrap()),
+            ]);
+            assert_eq!(*error.parameters(), expected);
+
+            server.shutdown().await;
+        })
+        .await
+}

witchcraft-server/src/blocking/conjure.rs

@@ -16,7 +16,8 @@ use crate::blocking::cancellation::CancellationGuard;
 use crate::blocking::pool::ThreadPool;
 use crate::blocking::{Cancellation, RequestBody, ResponseWriter};
 use crate::body::ClientIo;
-use crate::endpoint::{errors, WitchcraftEndpoint};
+use crate::endpoint::errors::ErrorConverter;
+use crate::endpoint::WitchcraftEndpoint;
 use crate::health::endpoint_500s::EndpointHealth;
 use crate::server::RawBody;
 use crate::service::endpoint_metrics::EndpointMetrics;
@@ -118,13 +119,14 @@ impl WitchcraftEndpoint for ConjureBlockingEndpoint {
             mdc::set(snapshot);
 
             let req = req.map(|inner| RequestBody::new(inner, handle.clone()));
+            let error_converter = ErrorConverter::new(req.headers());
             let mut response_extensions = Extensions::new();
 
             let mut response = match panic::catch_unwind(AssertUnwindSafe(|| {
                 endpoint.handle(req, &mut response_extensions)
             })) {
                 Ok(Ok(resp)) => resp,
-                Ok(Err(e)) => errors::to_response(&response_extensions, e, |o| {
+                Ok(Err(e)) => error_converter.convert(&response_extensions, e, |o| {
                     o.map_or(server::ResponseBody::Empty, server::ResponseBody::Fixed)
                 }),
                 Err(_) => {

witchcraft-server/src/endpoint/conjure.rs

@@ -11,7 +11,8 @@
 // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 // See the License for the specific language governing permissions and
 // limitations under the License.
-use crate::endpoint::{errors, WitchcraftEndpoint};
+use crate::endpoint::errors::ErrorConverter;
+use crate::endpoint::WitchcraftEndpoint;
 use crate::health::endpoint_500s::EndpointHealth;
 use crate::server::RawBody;
 use crate::service::endpoint_metrics::EndpointMetrics;
@@ -99,14 +100,15 @@ impl WitchcraftEndpoint for ConjureEndpoint {
 
     async fn handle(&self, req: Request<RawBody>) -> Response<BoxBody<Bytes, BodyWriteAborted>> {
         let req = req.map(RequestBody::new);
+        let error_converter = ErrorConverter::new(req.headers());
         let mut response_extensions = Extensions::new();
 
         let mut response = match AssertUnwindSafe(self.inner.handle(req, &mut response_extensions))
             .catch_unwind()
             .await
         {
             Ok(Ok(response)) => response.map(ResponseBody::new),
-            Ok(Err(error)) => errors::to_response(&response_extensions, error, |o| {
+            Ok(Err(error)) => error_converter.convert(&response_extensions, error, |o| {
                 o.map_or(
                     ResponseBody {
                         state: State::Empty,

witchcraft-server/src/endpoint/errors.rs

@@ -18,61 +18,96 @@ use conjure_error::{Error, ErrorKind};
 use conjure_http::server::UseLegacyErrorSerialization;
 use conjure_serde::json;
 use http::header::{CONTENT_TYPE, RETRY_AFTER};
-use http::{Extensions, HeaderValue, Response, StatusCode};
+use http::{Extensions, HeaderMap, HeaderName, HeaderValue, Response, StatusCode};
 use witchcraft_log::error;
 
-#[allow(clippy::declare_interior_mutable_const)]
 const APPLICATION_JSON: HeaderValue = HeaderValue::from_static("application/json");
+const ACCEPT_CONJURE_ERROR_PARAMETER_FORMAT: HeaderName =
+    HeaderName::from_static("accept-conjure-error-parameter-format");
+const JSON_FORMAT: HeaderValue = HeaderValue::from_static("JSON");
 
-pub fn to_response<F, B>(
-    response_extensions: &Extensions,
-    error: Error,
-    body_creator: F,
-) -> Response<B>
-where
-    F: FnOnce(Option<Bytes>) -> B,
-{
-    let mut response = match error.kind() {
-        ErrorKind::Service(service) => {
-            let body = if response_extensions
-                .get::<UseLegacyErrorSerialization>()
-                .is_some()
-            {
-                let service = conjure_error::stringify_parameters(service.clone());
-                json::to_vec(&service).unwrap()
-            } else {
-                json::to_vec(service).unwrap()
-            };
-            let mut response = Response::new(body_creator(Some(Bytes::from(body))));
-            *response.status_mut() =
-                StatusCode::from_u16(service.error_code().status_code()).unwrap();
-            response
-                .headers_mut()
-                .insert(CONTENT_TYPE, APPLICATION_JSON);
-            response
+pub struct ErrorConverter {
+    client_requested_json: bool,
+}
+
+impl ErrorConverter {
+    pub fn new(request_headers: &HeaderMap) -> Self {
+        ErrorConverter {
+            client_requested_json: Self::client_requested_json(request_headers),
         }
-        ErrorKind::Throttle(throttle) => {
-            let mut response = Response::new(body_creator(None));
-            *response.status_mut() = StatusCode::TOO_MANY_REQUESTS;
-            if let Some(duration) = throttle.duration() {
-                let header = HeaderValue::from(duration.as_secs());
-                response.headers_mut().insert(RETRY_AFTER, header);
+    }
+
+    fn client_requested_json(request_headers: &HeaderMap) -> bool {
+        if let Some(format) = request_headers.get(ACCEPT_CONJURE_ERROR_PARAMETER_FORMAT) {
+            if format
+                .as_bytes()
+                .eq_ignore_ascii_case(JSON_FORMAT.as_bytes())
+            {
+                return true;
             }
-            response
-        }
-        ErrorKind::Unavailable(_) => {
-            let mut response = Response::new(body_creator(None));
-            *response.status_mut() = StatusCode::SERVICE_UNAVAILABLE;
-            response
         }
-        _ => {
-            error!("unknown error kind");
-            let mut response = Response::new(body_creator(None));
-            *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR;
-            response
+
+        false
+    }
+
+    fn should_stringify_parameters(&self, response_extensions: &Extensions) -> bool {
+        if self.client_requested_json {
+            return false;
         }
-    };
 
-    response.extensions_mut().insert(Arc::new(error));
-    response
+        response_extensions
+            .get::<UseLegacyErrorSerialization>()
+            .is_some()
+    }
+
+    pub fn convert<F, B>(
+        self,
+        response_extensions: &Extensions,
+        error: Error,
+        body_creator: F,
+    ) -> Response<B>
+    where
+        F: FnOnce(Option<Bytes>) -> B,
+    {
+        let mut response = match error.kind() {
+            ErrorKind::Service(service) => {
+                let body = if self.should_stringify_parameters(response_extensions) {
+                    let service = conjure_error::stringify_parameters(service.clone());
+                    json::to_vec(&service).unwrap()
+                } else {
+                    json::to_vec(service).unwrap()
+                };
+                let mut response = Response::new(body_creator(Some(Bytes::from(body))));
+                *response.status_mut() =
+                    StatusCode::from_u16(service.error_code().status_code()).unwrap();
+                response
+                    .headers_mut()
+                    .insert(CONTENT_TYPE, APPLICATION_JSON);
+                response
+            }
+            ErrorKind::Throttle(throttle) => {
+                let mut response = Response::new(body_creator(None));
+                *response.status_mut() = StatusCode::TOO_MANY_REQUESTS;
+                if let Some(duration) = throttle.duration() {
+                    let header = HeaderValue::from(duration.as_secs());
+                    response.headers_mut().insert(RETRY_AFTER, header);
+                }
+                response
+            }
+            ErrorKind::Unavailable(_) => {
+                let mut response = Response::new(body_creator(None));
+                *response.status_mut() = StatusCode::SERVICE_UNAVAILABLE;
+                response
+            }
+            _ => {
+                error!("unknown error kind");
+                let mut response = Response::new(body_creator(None));
+                *response.status_mut() = StatusCode::INTERNAL_SERVER_ERROR;
+                response
+            }
+        };
+
+        response.extensions_mut().insert(Arc::new(error));
+        response
+    }
 }