Skip to content

Commit

Permalink
Add path to IsolateOutOfMemory error (#33252)
Browse files Browse the repository at this point in the history
GitOrigin-RevId: dd53c0075d94e2c189c7c6695b5078e8eaa5eafa
  • Loading branch information
emmaling27 authored and Convex, Inc. committed Jan 16, 2025
1 parent 84679db commit dbaa90c
Show file tree
Hide file tree
Showing 8 changed files with 21 additions and 10 deletions.
11 changes: 9 additions & 2 deletions crates/isolate/src/environment/action/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,8 @@ impl<RT: Runtime> ActionEnvironment<RT> {
)
.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
Expand Down Expand Up @@ -652,7 +653,13 @@ impl<RT: Runtime> ActionEnvironment<RT> {
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));
Expand Down
2 changes: 1 addition & 1 deletion crates/isolate/src/environment/analyze.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion crates/isolate/src/environment/auth_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 2 additions & 2 deletions crates/isolate/src/environment/component_definitions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand Down Expand Up @@ -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)
}
Expand Down
2 changes: 1 addition & 1 deletion crates/isolate/src/environment/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
4 changes: 3 additions & 1 deletion crates/isolate/src/environment/udf/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -389,6 +389,7 @@ impl<RT: Runtime> DatabaseUdfEnvironment<RT> {
// 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);
Expand All @@ -406,7 +407,8 @@ impl<RT: Runtime> DatabaseUdfEnvironment<RT> {
*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)) => {
Expand Down
4 changes: 3 additions & 1 deletion crates/isolate/src/termination.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@ impl IsolateHandle {
pub fn take_termination_error(
&self,
heap_stats: Option<IsolateHeapStats>,
// The isolate environment and function path (if applicable)
source: &str,
) -> anyhow::Result<Result<(), JsError>> {
let mut inner = self.inner.lock();
match &mut inner.reason {
Expand Down Expand Up @@ -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());
Expand Down
2 changes: 1 addition & 1 deletion crates/simulation/src/test_helpers/js_client/go.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
}
}

0 comments on commit dbaa90c

Please sign in to comment.