From dbaa90c81e67c032e6a244d87c40a97368ef828d Mon Sep 17 00:00:00 2001 From: Emma Forman Ling Date: Thu, 16 Jan 2025 14:24:35 -0800 Subject: [PATCH] Add path to IsolateOutOfMemory error (#33252) GitOrigin-RevId: dd53c0075d94e2c189c7c6695b5078e8eaa5eafa --- crates/isolate/src/environment/action/mod.rs | 11 +++++++++-- crates/isolate/src/environment/analyze.rs | 2 +- crates/isolate/src/environment/auth_config.rs | 2 +- .../isolate/src/environment/component_definitions.rs | 4 ++-- crates/isolate/src/environment/schema.rs | 2 +- crates/isolate/src/environment/udf/mod.rs | 4 +++- crates/isolate/src/termination.rs | 4 +++- crates/simulation/src/test_helpers/js_client/go.rs | 2 +- 8 files changed, 21 insertions(+), 10 deletions(-) diff --git a/crates/isolate/src/environment/action/mod.rs b/crates/isolate/src/environment/action/mod.rs index 4c58c239..53698431 100644 --- a/crates/isolate/src/environment/action/mod.rs +++ b/crates/isolate/src/environment/action/mod.rs @@ -350,7 +350,8 @@ impl ActionEnvironment { ) .await; // Override the returned result if we hit a termination error. - let termination_error = handle.take_termination_error(Some(heap_stats.get())); + let termination_error = handle + .take_termination_error(Some(heap_stats.get()), &format!("http action: {udf_path}")); // Perform a microtask checkpoint one last time before taking the environment // to ensure the microtask queue is empty. Otherwise, JS from this request may @@ -652,7 +653,13 @@ impl ActionEnvironment { isolate_context.scope.perform_microtask_checkpoint(); *isolate_clean = true; - match handle.take_termination_error(Some(heap_stats.get())) { + match handle.take_termination_error( + Some(heap_stats.get()), + &format!( + "{:?}", + request_params.path_and_args.path().clone().for_logging() + ), + ) { Ok(Ok(..)) => (), Ok(Err(e)) => { result = Ok(Err(e)); diff --git a/crates/isolate/src/environment/analyze.rs b/crates/isolate/src/environment/analyze.rs index b7249801..c474e925 100644 --- a/crates/isolate/src/environment/analyze.rs +++ b/crates/isolate/src/environment/analyze.rs @@ -299,7 +299,7 @@ impl AnalyzeEnvironment { drop(isolate_context); // Suppress the original error if the isolate was forcibly terminated. - if let Err(e) = handle.take_termination_error(None)? { + if let Err(e) = handle.take_termination_error(None, "analyze")? { return Ok(Err(e)); } result diff --git a/crates/isolate/src/environment/auth_config.rs b/crates/isolate/src/environment/auth_config.rs index c5c878f8..6e8b9b0c 100644 --- a/crates/isolate/src/environment/auth_config.rs +++ b/crates/isolate/src/environment/auth_config.rs @@ -232,7 +232,7 @@ impl AuthConfigEnvironment { // the next request starts, the isolate may panic. drop(isolate_context); - handle.take_termination_error(None)??; + handle.take_termination_error(None, "auth")??; result } diff --git a/crates/isolate/src/environment/component_definitions.rs b/crates/isolate/src/environment/component_definitions.rs index 52fe3c4e..d412dc15 100644 --- a/crates/isolate/src/environment/component_definitions.rs +++ b/crates/isolate/src/environment/component_definitions.rs @@ -292,7 +292,7 @@ impl AppDefinitionEvaluator { isolate_context.scope.perform_microtask_checkpoint(); drop(isolate_context); - handle.take_termination_error(None)??; + handle.take_termination_error(None, "evaluate_definition")??; Ok(result) } @@ -435,7 +435,7 @@ impl ComponentInitializerEvaluator { isolate_context.scope.perform_microtask_checkpoint(); drop(isolate_context); - handle.take_termination_error(None)??; + handle.take_termination_error(None, "evaluate")??; Ok(result) } diff --git a/crates/isolate/src/environment/schema.rs b/crates/isolate/src/environment/schema.rs index a4121900..073db2cc 100644 --- a/crates/isolate/src/environment/schema.rs +++ b/crates/isolate/src/environment/schema.rs @@ -211,7 +211,7 @@ impl SchemaEnvironment { // the next request starts, the isolate may panic. drop(isolate_context); - handle.take_termination_error(None)??; + handle.take_termination_error(None, "schema")??; result } diff --git a/crates/isolate/src/environment/udf/mod.rs b/crates/isolate/src/environment/udf/mod.rs index 6b954db6..9ad56d89 100644 --- a/crates/isolate/src/environment/udf/mod.rs +++ b/crates/isolate/src/environment/udf/mod.rs @@ -389,6 +389,7 @@ impl DatabaseUdfEnvironment { // that method directly since we want an `await` below, and passing in a // generic async closure to `Isolate` is currently difficult. let client_id = Arc::new(client_id); + let path = self.path.clone(); let (handle, state) = isolate.start_request(client_id, self).await?; let mut handle_scope = isolate.handle_scope(); let v8_context = v8::Context::new(&mut handle_scope); @@ -406,7 +407,8 @@ impl DatabaseUdfEnvironment { *isolate_clean = true; // Override the returned result if we hit a termination error. - let termination_error = handle.take_termination_error(Some(heap_stats.get())); + let termination_error = handle + .take_termination_error(Some(heap_stats.get()), &format!("{:?}", path.for_logging())); match termination_error { Ok(Ok(..)) => (), Ok(Err(e)) => { diff --git a/crates/isolate/src/termination.rs b/crates/isolate/src/termination.rs index 53aeae3a..1c04aadd 100644 --- a/crates/isolate/src/termination.rs +++ b/crates/isolate/src/termination.rs @@ -119,6 +119,8 @@ impl IsolateHandle { pub fn take_termination_error( &self, heap_stats: Option, + // The isolate environment and function path (if applicable) + source: &str, ) -> anyhow::Result> { let mut inner = self.inner.lock(); match &mut inner.reason { @@ -151,7 +153,7 @@ impl IsolateHandle { "IsolateOutOfMemory", format!( "Isolate ran out of memory during execution with heap stats: \ - {heap_stats:?}" + {heap_stats:?} in {source:?}" ), ); report_error_sync(&mut error.into()); diff --git a/crates/simulation/src/test_helpers/js_client/go.rs b/crates/simulation/src/test_helpers/js_client/go.rs index 22a01057..8e0eadb4 100644 --- a/crates/simulation/src/test_helpers/js_client/go.rs +++ b/crates/simulation/src/test_helpers/js_client/go.rs @@ -102,7 +102,7 @@ impl JsClientThread { } tracing::info!("JsThread shutting down..."); drop(isolate_context); - handle.take_termination_error(None)??; + handle.take_termination_error(None, "test")??; Ok(()) } }