Merge pull request #34 from Virtual-Repetitions/enum-params

scoop1120 · web-flow · commit 06131de80594 · 2026-02-07T16:35:17.000-08:00
Add Sentry integration and enum input validation
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -1,5 +1,5 @@
 [workspace.package]
-version = "0.29.8"
+version = "0.29.9"
 edition = "2024"
 
 # See https://github.com/mozilla/application-services/blob/main/Cargo.toml for the reasons why we use this structure
@@ -133,6 +133,7 @@ pluralizer = "0.5.0"
 tracing-subscriber = "0.3.20"
 toml = { version = "0.9.5", features = ["parse"] }
 zip = "6.0.0"
+sentry = "0.32.0"
 
 # reduce binary size, does not affect stack traces
 [profile.dev]
diff --git a/crates/core-subsystem/core-resolver/src/validation/document_validator.rs b/crates/core-subsystem/core-resolver/src/validation/document_validator.rs
@@ -303,6 +303,36 @@ mod tests {
         );
     }
 
+    #[cfg_attr(not(target_family = "wasm"), tokio::test)]
+    #[cfg_attr(target_family = "wasm", wasm_bindgen_test::wasm_bindgen_test)]
+    async fn enum_in_input_object_variable_is_valid() {
+        let schema = create_enum_test_schema().await;
+
+        let variables = create_variables(
+            r#"
+            {
+                "data": {
+                    "title": "EnumVar",
+                    "priority": "LOW"
+                }
+            }"#,
+        );
+
+        let validator = DocumentValidator::new(&schema, None, Some(variables), 10, 10);
+
+        let query = r#"
+            mutation($data: TaskCreationInput!) {
+                createTask(data: $data) {
+                    id
+                    priority
+                }
+            }
+        "#;
+
+        let result = validator.validate(create_query_document(query));
+        assert!(result.is_ok(), "{result:?}");
+    }
+
     #[cfg_attr(not(target_family = "wasm"), tokio::test)]
     #[cfg_attr(target_family = "wasm", wasm_bindgen_test::wasm_bindgen_test)]
     async fn invalid_subfield() {
@@ -837,6 +867,44 @@ mod tests {
         )
     }
 
+    async fn create_enum_test_schema() -> Schema {
+        let test_exo = r#"
+            @postgres
+            module LogModule {
+                @access(true)
+                type Task {
+                    @pk id: Int = autoIncrement()
+                    title: String
+                    priority: Priority
+                }
+
+                enum Priority {
+                    LOW
+                    MEDIUM
+                    HIGH
+                }
+            }
+        "#;
+
+        let postgres_subsystem =
+            create_postgres_system_from_str(test_exo, "enum-test.exo".to_string()).await;
+
+        Schema::new(
+            postgres_subsystem.graphql.as_ref().unwrap().schema_types(),
+            postgres_subsystem
+                .graphql
+                .as_ref()
+                .unwrap()
+                .schema_queries(),
+            postgres_subsystem
+                .graphql
+                .as_ref()
+                .unwrap()
+                .schema_mutations(),
+            Arc::new(None),
+        )
+    }
+
     fn create_query_document(query_str: &str) -> ExecutableDocument {
         parse_query(query_str).unwrap()
     }
diff --git a/crates/core-subsystem/core-resolver/src/validation/operation_validator.rs b/crates/core-subsystem/core-resolver/src/validation/operation_validator.rs
@@ -16,7 +16,7 @@ use async_graphql_parser::{
         VariableDefinition,
     },
 };
-use async_graphql_value::{ConstValue, Name};
+use async_graphql_value::{ConstValue, Name, indexmap::IndexMap};
 use serde::de::Error;
 use serde_json::{Map, Value};
 
@@ -226,33 +226,77 @@ impl<'a> OperationValidator<'a> {
                 }),
             },
             BaseType::Named(type_name) => {
-                if let Some(type_definition) = self.schema.get_type_definition(type_name.as_str())
-                    && let TypeKind::Enum(enum_type) = &type_definition.kind
-                {
-                    return match value {
-                        Value::String(enum_value) => {
-                            let is_valid = enum_type
-                                .values
-                                .iter()
-                                .any(|value_def| value_def.node.value.node.as_str() == enum_value);
+                if let Some(type_definition) = self.schema.get_type_definition(type_name.as_str()) {
+                    match &type_definition.kind {
+                        TypeKind::Enum(enum_type) => {
+                            return match value {
+                                Value::String(enum_value) => {
+                                    let is_valid = enum_type.values.iter().any(|value_def| {
+                                        value_def.node.value.node.as_str() == enum_value
+                                    });
 
-                            if is_valid {
-                                Ok(ConstValue::Enum(Name::new(enum_value)))
-                            } else {
-                                Err(error(format!(
-                                    "Invalid enum value '{}' for type '{}'",
-                                    enum_value,
-                                    type_name.as_str()
-                                )))
-                            }
+                                    if is_valid {
+                                        Ok(ConstValue::Enum(Name::new(enum_value)))
+                                    } else {
+                                        Err(error(format!(
+                                            "Invalid enum value '{}' for type '{}'",
+                                            enum_value,
+                                            type_name.as_str()
+                                        )))
+                                    }
+                                }
+                                Value::Null => Ok(ConstValue::Null),
+                                _ => Err(error(format!(
+                                    "Expected enum value for type '{}', got {}",
+                                    type_name.as_str(),
+                                    value
+                                ))),
+                            };
                         }
-                        Value::Null => Ok(ConstValue::Null),
-                        _ => Err(error(format!(
-                            "Expected enum value for type '{}', got {}",
-                            type_name.as_str(),
-                            value
-                        ))),
-                    };
+                        TypeKind::InputObject(input_object_type) => {
+                            return match value {
+                                Value::Object(values) => {
+                                    let coerced_values = values
+                                        .into_iter()
+                                        .map(|(field_name, field_value)| {
+                                            let field_definition =
+                                                input_object_type.fields.iter().find(|field| {
+                                                    field.node.name.node.as_str() == field_name
+                                                });
+
+                                            let coerced_value = match field_definition {
+                                                Some(field_definition) => self
+                                                    .coerce_variable_value(
+                                                        &field_definition.node.ty.node,
+                                                        field_value,
+                                                        variable_name,
+                                                    )?,
+                                                None => ConstValue::from_json(field_value)
+                                                    .map_err(|e| {
+                                                        ValidationError::MalformedVariable(
+                                                            variable_name.node.as_str().to_string(),
+                                                            variable_name.pos,
+                                                            e,
+                                                        )
+                                                    })?,
+                                            };
+
+                                            Ok((Name::new(field_name), coerced_value))
+                                        })
+                                        .collect::<Result<IndexMap<_, _>, ValidationError>>()?;
+
+                                    Ok(ConstValue::Object(coerced_values))
+                                }
+                                Value::Null => Ok(ConstValue::Null),
+                                _ => Err(error(format!(
+                                    "Expected input object for type '{}', got {}",
+                                    type_name.as_str(),
+                                    value
+                                ))),
+                            };
+                        }
+                        _ => {}
+                    }
                 }
 
                 ConstValue::from_json(value).map_err(|e| {
diff --git a/crates/graphql-router/Cargo.toml b/crates/graphql-router/Cargo.toml
@@ -18,6 +18,7 @@ core-resolver = { path = "../core-subsystem/core-resolver" }
 introspection-resolver = { path = "../introspection-subsystem/introspection-resolver" }
 common = { path = "../common" }
 exo-env = { path = "../../libs/exo-env" }
+sentry.workspace = true
 
 [dev-dependencies]
 
diff --git a/crates/graphql-router/src/graphql_router.rs b/crates/graphql-router/src/graphql_router.rs
@@ -23,6 +23,7 @@ use core_resolver::introspection::definition::schema::Schema;
 use core_resolver::plugin::SubsystemGraphQLResolver;
 use core_router::SystemLoadingError;
 use http::StatusCode;
+use sentry::Level;
 
 use ::tracing::instrument;
 use async_graphql_parser::Pos;
@@ -83,6 +84,39 @@ impl GraphQLRouter {
     }
 }
 
+fn capture_graphql_error(
+    err: &SystemResolutionError,
+    request_context: &RequestContext<'_>,
+    status_code: StatusCode,
+) {
+    if sentry::Hub::current().client().is_none() {
+        return;
+    }
+
+    let request_head = request_context.get_head();
+    let message = err.user_error_message();
+
+    sentry::with_scope(
+        |scope| {
+            scope.set_tag("graphql.path", request_head.get_path());
+            scope.set_tag("http.method", request_head.get_method().as_str());
+            scope.set_tag("error.kind", format!("{:?}", err));
+            scope.set_tag(
+                "internal_request",
+                request_context.is_internal().to_string(),
+            );
+            scope.set_tag(
+                "auth_present",
+                request_context.is_authentication_info_present().to_string(),
+            );
+            scope.set_extra("status_code", status_code.as_u16().into());
+        },
+        || {
+            sentry::capture_message(&message, Level::Error);
+        },
+    );
+}
+
 #[async_trait]
 impl<'a> Router<RequestContext<'a>> for GraphQLRouter {
     /// Resolves an incoming query, returning a response stream containing JSON and a set
@@ -125,13 +159,20 @@ impl<'a> Router<RequestContext<'a>> for GraphQLRouter {
         )
         .await;
 
-        if let Err(SystemResolutionError::RequestError(e)) = response {
-            tracing::error!("Error while resolving request: {:?}", e);
-            return Some(ResponsePayload {
-                body: ResponseBody::None,
-                headers: Headers::new(),
-                status_code: StatusCode::BAD_REQUEST,
-            });
+        match &response {
+            Err(err @ SystemResolutionError::RequestError(e)) => {
+                tracing::error!("Error while resolving request: {:?}", e);
+                capture_graphql_error(err, request_context, StatusCode::BAD_REQUEST);
+                return Some(ResponsePayload {
+                    body: ResponseBody::None,
+                    headers: Headers::new(),
+                    status_code: StatusCode::BAD_REQUEST,
+                });
+            }
+            Err(err) => {
+                capture_graphql_error(err, request_context, StatusCode::OK);
+            }
+            Ok(_) => {}
         }
 
         let mut headers = if let Ok(ref response) = response {
diff --git a/crates/server-common/Cargo.toml b/crates/server-common/Cargo.toml
@@ -17,6 +17,7 @@ postgres-resolver = { path = "../postgres-subsystem/postgres-resolver", features
 deno-resolver = { path = "../deno-subsystem/deno-resolver", optional = true }
 wasm-resolver = { path = "../wasm-subsystem/wasm-resolver", optional = true }
 exo-env = { path = "../../libs/exo-env" }
+sentry.workspace = true
 
 [features]
 static-postgres-resolver = ["postgres-resolver"]
diff --git a/crates/server-common/src/lib.rs b/crates/server-common/src/lib.rs
@@ -12,6 +12,8 @@ use std::{env, process::exit, sync::Arc};
 use thiserror::Error;
 
 use common::logging_tracing::{self, OtelError};
+
+mod sentry;
 use core_plugin_interface::interface::SubsystemLoader;
 
 use core_router::SystemLoadingError;
@@ -29,6 +31,7 @@ use system_router::{SystemRouter, create_system_router_from_file};
 /// - 1 - If the exo_ir file doesn't exist or can't be loaded.
 pub async fn init(env: Arc<dyn Environment>) -> Result<SystemRouter, ServerInitError> {
     logging_tracing::init(env.as_ref()).await?;
+    sentry::init(env.as_ref());
 
     println!(
         "Exograph server starting (version {})",
diff --git a/crates/server-common/src/sentry.rs b/crates/server-common/src/sentry.rs
@@ -0,0 +1,56 @@
+use exo_env::Environment;
+use std::sync::OnceLock;
+
+static SENTRY_GUARD: OnceLock<sentry::ClientInitGuard> = OnceLock::new();
+
+fn get_env_value(env: &dyn Environment, key: &str) -> Option<String> {
+    env.get(key)
+        .map(|value| value.trim().to_string())
+        .filter(|v| !v.is_empty())
+}
+
+pub fn init(env: &dyn Environment) {
+    if SENTRY_GUARD.get().is_some() {
+        return;
+    }
+
+    let dsn = get_env_value(env, "EXO_SENTRY_DSN").or_else(|| get_env_value(env, "SENTRY_DSN"));
+
+    let Some(dsn) = dsn else {
+        return;
+    };
+
+    let environment = get_env_value(env, "EXO_SENTRY_ENVIRONMENT")
+        .or_else(|| get_env_value(env, "SENTRY_ENVIRONMENT"))
+        .or_else(|| get_env_value(env, "EXO_ENV"));
+
+    let release =
+        get_env_value(env, "EXO_SENTRY_RELEASE").or_else(|| get_env_value(env, "SENTRY_RELEASE"));
+
+    let sample_rate = get_env_value(env, "EXO_SENTRY_SAMPLE_RATE")
+        .or_else(|| get_env_value(env, "SENTRY_SAMPLE_RATE"))
+        .and_then(|value| value.parse::<f32>().ok())
+        .unwrap_or(1.0);
+
+    let traces_sample_rate = get_env_value(env, "EXO_SENTRY_TRACES_SAMPLE_RATE")
+        .or_else(|| get_env_value(env, "SENTRY_TRACES_SAMPLE_RATE"))
+        .and_then(|value| value.parse::<f32>().ok())
+        .unwrap_or(0.0);
+
+    let options = sentry::ClientOptions {
+        dsn: dsn.parse().ok(),
+        environment: environment.map(Into::into),
+        release: release.map(Into::into),
+        sample_rate,
+        traces_sample_rate,
+        ..Default::default()
+    };
+
+    let guard = sentry::init(options);
+    let _ = SENTRY_GUARD.set(guard);
+}
+
+#[allow(dead_code)]
+pub fn enabled() -> bool {
+    sentry::Hub::current().client().is_some()
+}
diff --git a/integration-tests/todo/tests/mutation-enum-variable.exotest b/integration-tests/todo/tests/mutation-enum-variable.exotest
