Fix system errors for cross-component calls for queries & mutations (…

…#28456)

GitOrigin-RevId: edf4bbc8595269d5420cda51e5fb4e0a27d41e03

crates/application/src/tests/components.rs

@@ -3,6 +3,7 @@ use common::{
         CanonicalizedComponentFunctionPath,
         ComponentPath,
     },
+    testing::assert_contains,
     types::{
         EnvironmentVariable,
         FunctionCaller,
@@ -160,3 +161,32 @@ async fn test_system_env_vars_not_accessible_in_components(rt: TestRuntime) -> a
     assert_eq!(ConvexValue::Null, result.value);
     Ok(())
 }
+
+#[convex_macro::test_runtime]
+async fn test_system_error_propagation(rt: TestRuntime) -> anyhow::Result<()> {
+    let application = Application::new_for_tests(&rt).await?;
+    application.load_component_tests_modules().await?;
+
+    // The system error from the subquery should propagate to the top-level query.
+    let error = run_function(
+        &application,
+        "errors:throwSystemErrorFromQuery".parse()?,
+        vec![],
+    )
+    .await
+    .unwrap_err();
+    assert_contains(&error, "I can't go for that");
+
+    // Actions throw a JS error into user space when a call to `ctx.runAction`
+    // throws a system error, so we don't propagate them here.
+    let result = run_function(
+        &application,
+        "errors:throwSystemErrorFromAction".parse()?,
+        vec![],
+    )
+    .await?
+    .unwrap_err();
+    assert_contains(&result.error, "Your request couldn't be completed");
+
+    Ok(())
+}

crates/common/src/testing/mod.rs

@@ -4,6 +4,8 @@ mod schema;
 mod test_id_generator;
 mod test_persistence;
 
+use std::fmt::Display;
+
 pub use cmd_util::env::config_test as init_test_logging;
 use proptest::{
     arbitrary::{
@@ -41,3 +43,10 @@ pub fn generate_with<T: Arbitrary>(args: T::Parameters) -> T {
         .expect("Failed to create value tree");
     tree.current()
 }
+
+pub fn assert_contains(error: &impl Display, expected: &str) {
+    assert!(
+        format!("{}", error).contains(expected),
+        "\nExpected: {expected}\nActual: {error}"
+    );
+}

crates/isolate/src/environment/action/syscall.rs

@@ -19,7 +19,7 @@ impl<RT: Runtime> ActionEnvironment<RT> {
         match name {
             "1.0/componentArgument" => syscall_component_argument(self, args),
 
-            #[cfg(test)]
+            #[cfg(any(test, feature = "testing"))]
             "throwSystemError" => anyhow::bail!("I can't go for that."),
             _ => {
                 anyhow::bail!(ErrorMetadata::bad_request(

crates/isolate/src/environment/udf/mod.rs

@@ -412,10 +412,16 @@ impl<RT: Runtime> DatabaseUdfEnvironment<RT> {
                 result = Err(e);
             },
         }
+        // Our environment may be in an inconsistent state after a system error (e.g.
+        // the transaction may be missing if we hit a system error during a
+        // cross-component call), so be sure to error out here before using the
+        // environment.
+        let result = result?;
+
         let execution_time;
         (self, execution_time) = isolate_context.take_environment();
         let success_result_value = match result.as_ref() {
-            Ok(Ok(v)) => Some(v),
+            Ok(v) => Some(v),
             _ => None,
         };
         Self::add_warnings_to_log_lines(
@@ -447,7 +453,7 @@ impl<RT: Runtime> DatabaseUdfEnvironment<RT> {
                 observed_time: self.phase.observed_time(),
                 log_lines: self.log_lines,
                 journal: self.next_journal,
-                result: match result? {
+                result: match result {
                     Ok(v) => Ok(JsonPackedValue::pack(v)),
                     Err(e) => Err(e),
                 },
@@ -466,7 +472,7 @@ impl<RT: Runtime> DatabaseUdfEnvironment<RT> {
                 observed_time: self.phase.observed_time(),
                 log_lines: self.log_lines,
                 journal: self.next_journal,
-                result: match result? {
+                result: match result {
                     Ok(v) => Ok(JsonPackedValue::pack(v)),
                     Err(e) => Err(e),
                 },

crates/isolate/src/environment/udf/syscall.rs

@@ -114,7 +114,7 @@ pub fn syscall_impl<RT: Runtime, P: SyscallProvider<RT>>(
         "1.0/db/normalizeId" => syscall_normalize_id(provider, args),
         "1.0/componentArgument" => syscall_component_argument(provider, args),
 
-        #[cfg(test)]
+        #[cfg(any(test, feature = "testing"))]
         "throwSystemError" => anyhow::bail!("I can't go for that."),
         "throwOcc" => anyhow::bail!(ErrorMetadata::user_occ(None, None)),
         "throwOverloaded" => {

crates/isolate/src/tests/action.rs

@@ -1,7 +1,10 @@
 use std::time::Duration;
 
 use common::{
-    testing::TestPersistence,
+    testing::{
+        assert_contains,
+        TestPersistence,
+    },
     version::Version,
 };
 use must_let::must_let;
@@ -17,7 +20,6 @@ use crate::{
         UdfTest,
         UdfTestConfig,
     },
-    tests::assert_contains,
     IsolateConfig,
 };
 

crates/isolate/src/tests/adversarial.rs

@@ -14,6 +14,7 @@ use common::{
         ComponentPath,
     },
     log_lines::TRUNCATED_LINE_SUFFIX,
+    testing::assert_contains,
     types::{
         AllowedVisibility,
         UdfType,
@@ -39,7 +40,6 @@ use value::{
     TableNamespace,
 };
 
-use super::assert_contains;
 use crate::{
     concurrency_limiter::ConcurrencyLimiter,
     environment::helpers::MAX_LOG_LINES,

crates/isolate/src/tests/args_validation.rs

@@ -1,6 +1,9 @@
 use common::{
     errors::JsError,
-    testing::TestPersistence,
+    testing::{
+        assert_contains,
+        TestPersistence,
+    },
 };
 use keybroker::Identity;
 use must_let::must_let;
@@ -10,7 +13,6 @@ use value::{
     ConvexValue,
 };
 
-use super::assert_contains;
 use crate::test_helpers::{
     UdfTest,
     UdfTestType,

crates/isolate/src/tests/async.rs

@@ -1,16 +1,14 @@
 use common::{
     assert_obj,
+    testing::assert_contains,
     value::ConvexValue,
 };
 use must_let::must_let;
 use runtime::testing::TestRuntime;
 
-use crate::{
-    test_helpers::{
-        UdfTest,
-        UdfTestType,
-    },
-    tests::assert_contains,
+use crate::test_helpers::{
+    UdfTest,
+    UdfTestType,
 };
 
 #[convex_macro::test_runtime]

crates/isolate/src/tests/backend_state.rs

@@ -1,4 +1,7 @@
-use common::runtime::Runtime;
+use common::{
+    runtime::Runtime,
+    testing::assert_contains,
+};
 use database::Database;
 use errors::ErrorMetadata;
 use futures::channel::mpsc;
@@ -14,12 +17,9 @@ use value::assert_obj;
 
 use crate::{
     test_helpers::UdfTest,
-    tests::{
-        assert_contains,
-        http_action::{
-            http_action_udf_test,
-            http_post_request,
-        },
+    tests::http_action::{
+        http_action_udf_test,
+        http_post_request,
     },
     HttpActionResponseStreamer,
 };

crates/isolate/src/tests/basic.rs

@@ -3,6 +3,7 @@
 use common::{
     assert_obj,
     document::CreationTime,
+    testing::assert_contains,
     types::FieldName,
     value::{
         ConvexObject,
@@ -14,7 +15,6 @@ use must_let::must_let;
 use runtime::testing::TestRuntime;
 use value::assert_val;
 
-use super::assert_contains;
 use crate::test_helpers::{
     UdfTest,
     UdfTestType,

crates/isolate/src/tests/fetch.rs

@@ -25,6 +25,7 @@ use common::{
         NoopRouteMapper,
     },
     runtime::Runtime,
+    testing::assert_contains,
 };
 use http::{
     Request,
@@ -42,12 +43,9 @@ use value::ConvexValue;
 
 use crate::{
     test_helpers::UdfTest,
-    tests::{
-        assert_contains,
-        http_action::{
-            http_post_request,
-            http_request,
-        },
+    tests::http_action::{
+        http_post_request,
+        http_request,
     },
 };
 

crates/isolate/src/tests/http_action.rs

@@ -7,7 +7,10 @@ use common::{
     assert_obj,
     errors::JsError,
     runtime::Runtime,
-    testing::TestPersistence,
+    testing::{
+        assert_contains,
+        TestPersistence,
+    },
     version::Version,
 };
 use futures::{
@@ -45,7 +48,6 @@ use crate::{
         UdfTest,
         UdfTestConfig,
     },
-    tests::assert_contains,
     HttpActionRequestHead,
     HttpActionResponseStreamer,
     HttpActionResult,

crates/isolate/src/tests/import.rs

@@ -1,5 +1,6 @@
 use std::collections::HashSet;
 
+use common::testing::assert_contains;
 use keybroker::Identity;
 use model::environment_variables::{
     types::EnvironmentVariable,
@@ -8,10 +9,7 @@ use model::environment_variables::{
 use runtime::testing::TestRuntime;
 use value::assert_obj;
 
-use crate::{
-    test_helpers::UdfTest,
-    tests::assert_contains,
-};
+use crate::test_helpers::UdfTest;
 
 #[convex_macro::test_runtime]
 async fn test_action_dynamic_import(rt: TestRuntime) -> anyhow::Result<()> {

crates/isolate/src/tests/js_builtins.rs

@@ -6,18 +6,16 @@ use common::{
         Runtime,
         RuntimeInstant,
     },
+    testing::assert_contains,
 };
 use must_let::must_let;
 use pretty_assertions::assert_eq;
 use runtime::testing::TestRuntime;
 use value::ConvexValue;
 
-use crate::{
-    test_helpers::{
-        UdfTest,
-        UdfTestType,
-    },
-    tests::assert_contains,
+use crate::test_helpers::{
+    UdfTest,
+    UdfTestType,
 };
 
 #[convex_macro::test_runtime]

crates/isolate/src/tests/logging.rs

@@ -1,14 +1,14 @@
-use common::assert_obj;
+use common::{
+    assert_obj,
+    testing::assert_contains,
+};
 use itertools::Itertools;
 use regex::Regex;
 use runtime::testing::TestRuntime;
 
-use crate::{
-    test_helpers::{
-        UdfTest,
-        UdfTestType,
-    },
-    tests::assert_contains,
+use crate::test_helpers::{
+    UdfTest,
+    UdfTestType,
 };
 
 /// Tests to ensure that our logging is reasonable for basic JS types.

crates/isolate/src/tests/mod.rs

@@ -1,5 +1,3 @@
-use std::fmt::Display;
-
 mod action;
 mod adversarial;
 mod analyze;
@@ -34,10 +32,3 @@ mod unicode;
 mod user_error;
 mod values;
 mod vector_search;
-
-pub fn assert_contains(error: &impl Display, expected: &str) {
-    assert!(
-        format!("{}", error).contains(expected),
-        "\nExpected: {expected}\nActual: {error}"
-    );
-}

crates/isolate/src/tests/query.rs

@@ -9,7 +9,10 @@ use common::{
     persistence::Persistence,
     query::Cursor,
     runtime::Runtime,
-    testing::TestPersistence,
+    testing::{
+        assert_contains,
+        TestPersistence,
+    },
     types::PersistenceVersion,
     value::{
         ConvexArray,
@@ -28,7 +31,6 @@ use value::{
     ConvexObject,
 };
 
-use super::assert_contains;
 use crate::{
     test_helpers::{
         UdfTest,

crates/isolate/src/tests/scheduler.rs

@@ -5,6 +5,7 @@ use std::time::Duration;
 use common::{
     assert_obj,
     runtime::Runtime,
+    testing::assert_contains,
     value::ConvexValue,
 };
 use keybroker::Identity;
@@ -16,7 +17,6 @@ use must_let::must_let;
 use rand::RngCore;
 use runtime::testing::TestRuntime;
 
-use super::assert_contains;
 use crate::{
     test_helpers::{
         UdfTest,