Lazily get source maps on node action errors (#32892)

Currently we get load modules on every node action, but we only use the modules for source maps that are used to make error messages helpful. This PR makes the modules lazy load only when the node action fails.

GitOrigin-RevId: 799e42ebacb1d7ddf844524b67fb7553c5e4abed

crates/application/src/application_function_runner/mod.rs

@@ -1268,19 +1268,32 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
                     component: path.component,
                     module_path: path.udf_path.module().clone(),
                 };
-                let module_version = self
-                    .module_cache
-                    .get_module(&mut tx, module_path.clone())
+                let component = path.component;
+                let module_metadata = match ModuleModel::new(&mut tx)
+                    .get_metadata(module_path.clone())
                     .await?
-                    .context("Missing a valid module_version")?;
+                {
+                    Some(r) => r,
+                    None => anyhow::bail!("Missing a valid module_version"),
+                };
+                let source_package = SourcePackageModel::new(&mut tx, component.into())
+                    .get(module_metadata.source_package_id)
+                    .await?;
+                let source_maps_callback = async {
+                    let module_version = self
+                        .module_cache
+                        .get_module_with_metadata(module_metadata, source_package)
+                        .await?;
+                    let mut source_maps = BTreeMap::new();
+                    if let Some(source_map) = module_version.source_map.clone() {
+                        source_maps.insert(module_path.module_path.clone(), source_map);
+                    }
+                    Ok(source_maps)
+                };
                 let _request_guard = self
                     .node_action_limiter
                     .acquire_permit_with_timeout(&self.runtime)
                     .await?;
-                let mut source_maps = BTreeMap::new();
-                if let Some(source_map) = module_version.source_map.clone() {
-                    source_maps.insert(module_path.module_path.clone(), source_map);
-                }
 
                 let source_package_id = module.source_package_id;
                 let source_package = SourcePackageModel::new(&mut tx, component.into())
@@ -1343,7 +1356,7 @@ impl<RT: Runtime> ApplicationFunctionRunner<RT> {
 
                 let node_outcome_future = self
                     .node_actions
-                    .execute(request, &source_maps, log_line_sender)
+                    .execute(request, log_line_sender, source_maps_callback)
                     .boxed();
                 let (mut node_outcome_result, log_lines) = run_function_and_collect_log_lines(
                     node_outcome_future,

crates/node_executor/src/executor.rs

@@ -1,5 +1,6 @@
 use std::{
     collections::BTreeMap,
+    future::Future,
     path::Path,
     sync::{
         Arc,
@@ -209,11 +210,14 @@ impl<RT: Runtime> Actions<RT> {
         self.executor.shutdown()
     }
 
+    #[rustfmt::skip]
     pub async fn execute(
         &self,
         request: ExecuteRequest,
-        source_maps: &BTreeMap<CanonicalizedModulePath, SourceMap>,
         log_line_sender: mpsc::UnboundedSender<LogLine>,
+        source_maps_callback: impl Future<Output = anyhow::Result<
+            BTreeMap<CanonicalizedModulePath, SourceMap>>>
+            + Send,
     ) -> anyhow::Result<NodeActionOutcome> {
         let path = request.path_and_args.path().clone();
         let timer = node_executor("execute");
@@ -283,7 +287,8 @@ impl<RT: Runtime> Actions<RT> {
                 frames,
                 ..
             } => {
-                let error = construct_js_error(message, name, data, frames, source_maps)?;
+                let source_maps = source_maps_callback.await?;
+                let error = construct_js_error(message, name, data, frames, &source_maps)?;
                 Err(error)
             },
         };

crates/node_executor/src/local.rs

@@ -181,6 +181,7 @@ impl NodeExecutor for LocalNodeExecutor {
 mod tests {
     use std::{
         collections::BTreeMap,
+        future::Future,
         sync::{
             Arc,
             LazyLock,
@@ -310,15 +311,18 @@ mod tests {
         }
     }
 
+    #[rustfmt::skip]
     async fn execute<RT: Runtime>(
         actions: &Actions<RT>,
         execute_request: ExecuteRequest,
-        source_maps: &BTreeMap<CanonicalizedModulePath, SourceMap>,
+        source_maps_callback: impl Future<Output = anyhow::Result<
+            BTreeMap<CanonicalizedModulePath, SourceMap>>>
+            + Send,
     ) -> anyhow::Result<(NodeActionOutcome, LogLines)> {
         let (log_line_sender, log_line_receiver) = mpsc::unbounded_channel();
         let execute_future = Box::pin(
             actions
-                .execute(execute_request, source_maps, log_line_sender)
+                .execute(execute_request, log_line_sender, source_maps_callback)
                 .fuse(),
         );
         let (result, log_lines) =
@@ -338,14 +342,17 @@ mod tests {
         )
     }
 
+    async fn empty_source_maps_callback(
+    ) -> anyhow::Result<BTreeMap<CanonicalizedModulePath, SourceMap>> {
+        Ok(BTreeMap::new())
+    }
+
     #[convex_macro::prod_rt_test]
     async fn test_success(rt: ProdRuntime) -> anyhow::Result<()> {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
 
-        let source_maps = BTreeMap::new();
-
         let numbers: ConvexArray = array![1f64.into(), 7f64.into()]?;
         let args = create_args(assert_obj!("numbers" => ConvexValue::Array(numbers)))?;
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
@@ -356,7 +363,7 @@ mod tests {
         let (response, _log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
 
@@ -370,7 +377,6 @@ mod tests {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:logHelloWorldAndReturn7".parse()?,
             array![],
@@ -379,7 +385,7 @@ mod tests {
         let (response, log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
 
@@ -401,7 +407,6 @@ mod tests {
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
         let identity = UserIdentity::test();
 
-        let source_maps = BTreeMap::new();
         let (response, _log_lines) = execute(
             &actions,
             ExecuteRequest {
@@ -419,7 +424,7 @@ mod tests {
                 context: ExecutionContext::new_for_test(),
                 encoded_parent_trace: None,
             },
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
 
@@ -441,7 +446,6 @@ mod tests {
             rt,
         );
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
 
         let args = create_args(assert_obj!(
             "name" =>  "getCounter.js"
@@ -456,7 +460,7 @@ mod tests {
                 ),
                 source_package,
             ),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
 
@@ -478,7 +482,6 @@ mod tests {
             rt,
         );
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let args = create_args(assert_obj!(
             "name" =>  "getCounter.js"
         ))?;
@@ -492,7 +495,7 @@ mod tests {
                 ),
                 source_package,
             ),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
 
@@ -519,6 +522,7 @@ mod tests {
                 )
             })
             .collect();
+        let source_maps_callback = async { Ok(source_maps) };
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:logAndThrowError".parse()?,
             array![],
@@ -527,7 +531,7 @@ mod tests {
         let (response, log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            source_maps_callback,
         )
         .await?;
 
@@ -564,6 +568,7 @@ mod tests {
                 )
             })
             .collect();
+        let source_maps_callback = async { Ok(source_maps) };
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:forgotAwait".parse()?,
             array![],
@@ -572,7 +577,7 @@ mod tests {
         let (response, log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            source_maps_callback,
         )
         .await?;
 
@@ -598,7 +603,6 @@ mod tests {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:hello".parse()?,
             array![],
@@ -607,7 +611,7 @@ mod tests {
         let (response, _log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
 
@@ -625,7 +629,6 @@ mod tests {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let mut environment_variables = BTreeMap::new();
         environment_variables.insert("TEST_NAME".parse()?, "TEST_VALUE".parse()?);
         let (response, _log_lines) = execute(
@@ -645,7 +648,7 @@ mod tests {
                 context: ExecutionContext::new_for_test(),
                 encoded_parent_trace: None,
             },
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
         assert_eq!(response.result?, ConvexValue::try_from("TEST_VALUE")?);
@@ -658,7 +661,6 @@ mod tests {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:sleepAnHour".parse()?,
             array![],
@@ -667,7 +669,7 @@ mod tests {
         let (response, log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
         // This should be hitting the user timeout in executor.ts, not the Node
@@ -693,7 +695,6 @@ mod tests {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:partialEscapeSequence".parse()?,
             array![],
@@ -702,7 +703,7 @@ mod tests {
         let err = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await
         .unwrap_err();
@@ -715,7 +716,6 @@ mod tests {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:workHardForAnHour".parse()?,
             array![],
@@ -724,7 +724,7 @@ mod tests {
         let (response, log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
         // Since this is a busy loop, we should be hitting the process timeout.
@@ -749,7 +749,6 @@ mod tests {
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
         let path_and_args = ValidatedPathAndArgs::new_for_tests(
             "node_actions.js:deadlock".parse()?,
             array![],
@@ -758,7 +757,7 @@ mod tests {
         let (response, _log_lines) = execute(
             &actions,
             execute_request(path_and_args, source_package),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
         assert_eq!(
@@ -973,7 +972,6 @@ export { hello };
         let storage = Arc::new(LocalDirStorage::new(rt.clone())?);
         let actions = create_actions(rt);
         let source_package = upload_modules(storage.clone(), TEST_SOURCE.clone()).await?;
-        let source_maps = BTreeMap::new();
 
         // First, try to execute an action with a syscall that fails. In this case,
         // we'll call into a query where the backend isn't actually running.
@@ -988,7 +986,7 @@ export { hello };
                 ),
                 source_package.clone(),
             ),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
         let syscall_trace = response.syscall_trace;
@@ -1011,7 +1009,7 @@ export { hello };
                 ),
                 source_package,
             ),
-            &source_maps,
+            empty_source_maps_callback(),
         )
         .await?;
         let syscall_trace = response.syscall_trace;