fix(grpc): show additional error info from expression (#1809)

Co-authored-by: Tushar Mathur <[email protected]>

src/blueprint/into_schema.rs

@@ -2,6 +2,7 @@ use std::borrow::Cow;
 use std::sync::Arc;
 
 use async_graphql::dynamic::{self, FieldFuture, FieldValue, SchemaBuilder};
+use async_graphql::ErrorExtensions;
 use async_graphql_value::ConstValue;
 use futures_util::TryFutureExt;
 use tracing::Instrument;
@@ -68,8 +69,10 @@ fn to_type(def: &Definition) -> dynamic::Type {
                                         let ctx: ResolverContext = ctx.into();
                                         let ctx = EvaluationContext::new(req_ctx, &ctx);
 
-                                        let const_value =
-                                            expr.eval(ctx, &Concurrent::Sequential).await?;
+                                        let const_value = expr
+                                            .eval(ctx, &Concurrent::Sequential)
+                                            .await
+                                            .map_err(|err| err.extend())?;
                                         let p = match const_value {
                                             ConstValue::List(a) => Some(FieldValue::list(a)),
                                             ConstValue::Null => FieldValue::NONE,

src/http/request_context.rs

@@ -15,6 +15,7 @@ use crate::graphql::GraphqlDataLoader;
 use crate::grpc;
 use crate::grpc::data_loader::GrpcDataLoader;
 use crate::http::{AppContext, DataLoaderRequest, HttpDataLoader};
+use crate::lambda::EvaluationError;
 use crate::runtime::TargetRuntime;
 
 #[derive(Setters)]
@@ -33,7 +34,7 @@ pub struct RequestContext {
     pub min_max_age: Arc<Mutex<Option<i32>>>,
     pub cache_public: Arc<Mutex<Option<bool>>>,
     pub runtime: TargetRuntime,
-    pub cache: AsyncCache<u64, ConstValue, String>,
+    pub cache: AsyncCache<u64, ConstValue, EvaluationError>,
 }
 
 impl RequestContext {

src/lambda/cache.rs

@@ -3,10 +3,11 @@ use std::num::NonZeroU64;
 use std::ops::Deref;
 use std::pin::Pin;
 
-use anyhow::Result;
 use async_graphql_value::ConstValue;
 
-use super::{Concurrent, Eval, EvaluationContext, Expression, ResolverContextLike};
+use super::{
+    Concurrent, Eval, EvaluationContext, EvaluationError, Expression, ResolverContextLike,
+};
 
 pub trait CacheKey<Ctx> {
     fn cache_key(&self, ctx: &Ctx) -> u64;
@@ -39,7 +40,7 @@ impl Eval for Cache {
         &'a self,
         ctx: EvaluationContext<'a, Ctx>,
         conc: &'a Concurrent,
-    ) -> Pin<Box<dyn Future<Output = Result<ConstValue>> + 'a + Send>> {
+    ) -> Pin<Box<dyn Future<Output = Result<ConstValue, EvaluationError>> + 'a + Send>> {
         Box::pin(async move {
             if let Expression::IO(io) = self.expr.deref() {
                 let key = io.cache_key(&ctx);

src/lambda/concurrent.rs

@@ -1,6 +1,8 @@
 use futures_util::stream::FuturesUnordered;
 use futures_util::{Future, StreamExt};
 
+use super::EvaluationError;
+
 ///
 /// Concurrent controls the concurrency of a fold or foreach operation on lists.
 /// It's a flag that is set based on operators that are applied on a list.
@@ -19,7 +21,7 @@ impl Concurrent {
         iter: impl Iterator<Item = F>,
         acc: B,
         f: impl Fn(B, A) -> anyhow::Result<B>,
-    ) -> anyhow::Result<B>
+    ) -> Result<B, EvaluationError>
     where
         F: Future<Output = A>,
     {
@@ -46,9 +48,9 @@ impl Concurrent {
         &self,
         iter: impl Iterator<Item = F>,
         f: impl Fn(A) -> B,
-    ) -> anyhow::Result<Vec<B>>
+    ) -> Result<Vec<B>, EvaluationError>
     where
-        F: Future<Output = anyhow::Result<A>>,
+        F: Future<Output = Result<A, EvaluationError>>,
     {
         self.fold(iter, vec![], |mut acc, val| {
             acc.push(f(val?));

src/lambda/eval.rs

@@ -1,9 +1,7 @@
 use core::future::Future;
 use std::pin::Pin;
 
-use anyhow::Result;
-
-use super::{Concurrent, EvaluationContext, ResolverContextLike};
+use super::{Concurrent, EvaluationContext, EvaluationError, ResolverContextLike};
 
 pub trait Eval<Output = async_graphql::Value>
 where
@@ -13,7 +11,7 @@ where
         &'a self,
         ctx: EvaluationContext<'a, Ctx>,
         conc: &'a Concurrent,
-    ) -> Pin<Box<dyn Future<Output = Result<Output>> + 'a + Send>>
+    ) -> Pin<Box<dyn Future<Output = Result<Output, EvaluationError>> + 'a + Send>>
     where
         Output: 'a;
 }

src/lambda/expression.rs

@@ -3,12 +3,12 @@ use std::fmt::{Debug, Display};
 use std::pin::Pin;
 use std::sync::Arc;
 
-use anyhow::{anyhow, Result};
 use async_graphql::ErrorExtensions;
 use async_graphql_value::ConstValue;
 use thiserror::Error;
 
 use super::{Concurrent, Eval, EvaluationContext, ResolverContextLike, IO};
+use crate::auth;
 use crate::blueprint::DynamicValue;
 use crate::json::JsonLike;
 use crate::lambda::cache::Cache;
@@ -67,10 +67,19 @@ pub enum EvaluationError {
     #[error("APIValidationError: {0:?}")]
     APIValidationError(Vec<String>),
 
-    #[error("ExprEvalError: {0:?}")]
+    #[error("ExprEvalError: {0}")]
     ExprEvalError(String),
+
+    #[error("DeserializeError: {0}")]
+    DeserializeError(String),
+
+    #[error("Authentication Failure: {0}")]
+    AuthError(auth::error::Error),
 }
 
+// TODO: remove conversion from anyhow and don't use anyhow to pass errors
+// since it loses potentially valuable information that could be later provided
+// in the error extensions
 impl From<anyhow::Error> for EvaluationError {
     fn from(value: anyhow::Error) -> Self {
         match value.downcast::<EvaluationError>() {
@@ -99,19 +108,19 @@ impl ErrorExtensions for EvaluationError {
                 grpc_status_details,
             } = self
             {
-                e.set("grpc_code", *grpc_code);
-                e.set("grpc_description", grpc_description);
-                e.set("grpc_status_message", grpc_status_message);
-                e.set("grpc_status_details", grpc_status_details.clone());
+                e.set("grpcCode", *grpc_code);
+                e.set("grpcDescription", grpc_description);
+                e.set("grpcStatusMessage", grpc_status_message);
+                e.set("grpcStatusDetails", grpc_status_details.clone());
             }
         })
     }
 }
 
 impl<'a> From<crate::valid::ValidationError<&'a str>> for EvaluationError {
-    fn from(_value: crate::valid::ValidationError<&'a str>) -> Self {
+    fn from(value: crate::valid::ValidationError<&'a str>) -> Self {
         EvaluationError::APIValidationError(
-            _value
+            value
                 .as_vec()
                 .iter()
                 .map(|e| e.message.to_owned())
@@ -120,6 +129,12 @@ impl<'a> From<crate::valid::ValidationError<&'a str>> for EvaluationError {
     }
 }
 
+impl From<auth::error::Error> for EvaluationError {
+    fn from(value: auth::error::Error) -> Self {
+        EvaluationError::AuthError(value)
+    }
+}
+
 impl Expression {
     pub fn and_then(self, next: Self) -> Self {
         Expression::Context(Context::PushArgs { expr: Box::new(self), and_then: Box::new(next) })
@@ -136,7 +151,7 @@ impl Eval for Expression {
         &'a self,
         ctx: EvaluationContext<'a, Ctx>,
         conc: &'a Concurrent,
-    ) -> Pin<Box<dyn Future<Output = Result<ConstValue>> + 'a + Send>> {
+    ) -> Pin<Box<dyn Future<Output = Result<ConstValue, EvaluationError>> + 'a + Send>> {
         Box::pin(async move {
             match self {
                 Expression::Context(op) => match op {
@@ -165,14 +180,13 @@ impl Eval for Expression {
                         .unwrap_or(&async_graphql::Value::Null)
                         .clone())
                 }
-                Expression::Dynamic(value) => value.render_value(&ctx),
+                Expression::Dynamic(value) => Ok(value.render_value(&ctx)),
                 Expression::Protect(expr) => {
                     ctx.request_ctx
                         .auth_ctx
                         .validate(ctx.request_ctx)
                         .await
-                        .to_result()
-                        .map_err(|e| anyhow!("Authentication Failure: {}", e.to_string()))?;
+                        .to_result()?;
                     expr.eval(ctx, conc).await
                 }
                 Expression::IO(operation) => operation.eval(ctx, conc).await,

src/lambda/io.rs

@@ -2,7 +2,7 @@ use core::future::Future;
 use std::pin::Pin;
 use std::sync::Arc;
 
-use anyhow::Result;
+use async_graphql::from_value;
 use async_graphql_value::ConstValue;
 use reqwest::Request;
 
@@ -49,20 +49,15 @@ impl Eval for IO {
         &'a self,
         ctx: super::EvaluationContext<'a, Ctx>,
         _conc: &'a super::Concurrent,
-    ) -> Pin<Box<dyn Future<Output = Result<ConstValue>> + 'a + Send>> {
+    ) -> Pin<Box<dyn Future<Output = Result<ConstValue, EvaluationError>> + 'a + Send>> {
         let key = self.cache_key(&ctx);
         Box::pin(async move {
             ctx.request_ctx
                 .cache
                 .get_or_eval(key, move || {
-                    Box::pin(async {
-                        self.eval_inner(ctx, _conc)
-                            .await
-                            .map_err(|err| err.to_string())
-                    })
+                    Box::pin(async { self.eval_inner(ctx, _conc).await })
                 })
                 .await
-                .map_err(|err| anyhow::anyhow!(err))
         })
     }
 }
@@ -72,7 +67,7 @@ impl IO {
         &'a self,
         ctx: super::EvaluationContext<'a, Ctx>,
         _conc: &'a super::Concurrent,
-    ) -> Pin<Box<dyn Future<Output = Result<ConstValue>> + 'a + Send>> {
+    ) -> Pin<Box<dyn Future<Output = Result<ConstValue, EvaluationError>> + 'a + Send>> {
         Box::pin(async move {
             match self {
                 IO::Http { req_template, dl_id, .. } => {
@@ -190,7 +185,7 @@ fn set_cookie_headers<'ctx, Ctx: ResolverContextLike<'ctx>>(
 async fn execute_raw_request<'ctx, Ctx: ResolverContextLike<'ctx>>(
     ctx: &EvaluationContext<'ctx, Ctx>,
     req: Request,
-) -> Result<Response<async_graphql::Value>> {
+) -> Result<Response<async_graphql::Value>, EvaluationError> {
     let response = ctx
         .request_ctx
         .runtime
@@ -207,12 +202,10 @@ async fn execute_raw_grpc_request<'ctx, Ctx: ResolverContextLike<'ctx>>(
     ctx: &EvaluationContext<'ctx, Ctx>,
     req: Request,
     operation: &ProtobufOperation,
-) -> Result<Response<async_graphql::Value>> {
-    Ok(
-        execute_grpc_request(&ctx.request_ctx.runtime, operation, req)
-            .await
-            .map_err(EvaluationError::from)?,
-    )
+) -> Result<Response<async_graphql::Value>, EvaluationError> {
+    execute_grpc_request(&ctx.request_ctx.runtime, operation, req)
+        .await
+        .map_err(EvaluationError::from)
 }
 
 async fn execute_grpc_request_with_dl<
@@ -227,7 +220,7 @@ async fn execute_grpc_request_with_dl<
     ctx: &EvaluationContext<'ctx, Ctx>,
     rendered: RenderedRequestTemplate,
     data_loader: Option<&DataLoader<grpc::DataLoaderRequest, Dl>>,
-) -> Result<Response<async_graphql::Value>> {
+) -> Result<Response<async_graphql::Value>, EvaluationError> {
     let headers = ctx
         .request_ctx
         .upstream
@@ -253,7 +246,7 @@ async fn execute_request_with_dl<
     ctx: &EvaluationContext<'ctx, Ctx>,
     req: Request,
     data_loader: Option<&DataLoader<DataLoaderRequest, Dl>>,
-) -> Result<Response<async_graphql::Value>> {
+) -> Result<Response<async_graphql::Value>, EvaluationError> {
     let headers = ctx
         .request_ctx
         .upstream
@@ -275,8 +268,9 @@ fn parse_graphql_response<'ctx, Ctx: ResolverContextLike<'ctx>>(
     ctx: &EvaluationContext<'ctx, Ctx>,
     res: Response<async_graphql::Value>,
     field_name: &str,
-) -> Result<async_graphql::Value> {
-    let res: async_graphql::Response = serde_json::from_value(res.body.into_json()?)?;
+) -> Result<async_graphql::Value, EvaluationError> {
+    let res: async_graphql::Response =
+        from_value(res.body).map_err(|err| EvaluationError::DeserializeError(err.to_string()))?;
 
     for error in res.errors {
         ctx.add_error(error);