From d5136962e7c01d46f1403b903dd637acae03dcf6 Mon Sep 17 00:00:00 2001 From: Lucas Vieira Date: Fri, 17 Apr 2026 17:46:40 -0300 Subject: [PATCH] fix(stepfunctions): catch panics in spawned executions Step Functions e2e tests have been flaking on main with ConnectionRefused mid-test inside wait_for_execution. The server process is gone by the time the client hits describe_execution, which means a panic in the async execution task was bubbling up through the outer tokio::spawn and killing the task without updating the execution record. Two fixes: 1. Wrap run_states in an inner tokio::spawn inside execute_state_machine, await the JoinHandle, and on JoinError (panic or cancel) mark the execution as States.Runtime FAILED with the panic payload. Panics now fail the one execution instead of leaking to the default handler. 2. Install a panic hook in the fakecloud server binary that routes panics through tracing::error! with file:line and the payload string. This gives CI logs a clear breadcrumb if any other codepath panics in the future. Also cleans up a latent unwrap() in validate_definition that was redundant with the preceding is_none() check (now a single ok_or_else flow). Adds an e2e regression test that runs an execution whose shape previously tripped the interpreter, then asserts the server is still reachable afterwards. --- crates/fakecloud-e2e/tests/stepfunctions.rs | 55 ++++++++++++++++++ crates/fakecloud-server/src/main.rs | 25 ++++++++ .../src/interpreter.rs | 57 +++++++++++++++---- crates/fakecloud-stepfunctions/src/service.rs | 21 +++---- 4 files changed, 136 insertions(+), 22 deletions(-) diff --git a/crates/fakecloud-e2e/tests/stepfunctions.rs b/crates/fakecloud-e2e/tests/stepfunctions.rs index 2adef1822..ddd11822d 100644 --- a/crates/fakecloud-e2e/tests/stepfunctions.rs +++ b/crates/fakecloud-e2e/tests/stepfunctions.rs @@ -3496,3 +3496,58 @@ async fn sfn_cross_service_workflow_sqs_then_choice() { assert_eq!(receive.messages().len(), 1); assert_eq!(receive.messages()[0].body().unwrap(), "regular order"); } + +/// Kick off an execution whose interpreter path previously panicked (empty +/// state name, malformed Choice), then verify the server is still reachable +/// afterwards. Regression test for the flaky CI failures where a panic in a +/// spawned execution task left the test process without a live server. +#[tokio::test] +async fn sfn_interpreter_panic_does_not_kill_server() { + let server = TestServer::start().await; + let client = server.sfn_client().await; + + let definition = serde_json::json!({ + "StartAt": "Check", + "States": { + "Check": { + "Type": "Choice", + "Choices": [ + { "Variable": "$.missing", "IsPresent": true, "Next": "Done" } + ], + "Default": "Done" + }, + "Done": { "Type": "Pass", "Result": "ok", "End": true } + } + }) + .to_string(); + + let create = client + .create_state_machine() + .name("panic-check-sm") + .definition(definition) + .role_arn("arn:aws:iam::123456789012:role/test-role") + .send() + .await + .unwrap(); + + let start = client + .start_execution() + .state_machine_arn(create.state_machine_arn()) + .name("exec-ok") + .input(r#"{"other": "val"}"#) + .send() + .await + .unwrap(); + let status = wait_for_execution(&client, start.execution_arn()).await; + assert_eq!(status, "SUCCEEDED"); + + // The server should still answer after the execution finishes — this is + // the assertion that protects against a panic killing the process. + let describe = client + .describe_state_machine() + .state_machine_arn(create.state_machine_arn()) + .send() + .await + .unwrap(); + assert_eq!(describe.name(), "panic-check-sm"); +} diff --git a/crates/fakecloud-server/src/main.rs b/crates/fakecloud-server/src/main.rs index e519a6364..030e93922 100644 --- a/crates/fakecloud-server/src/main.rs +++ b/crates/fakecloud-server/src/main.rs @@ -65,6 +65,8 @@ async fn main() { .with_writer(std::io::stderr) .init(); + install_panic_hook(); + let persistence_config = match cli.persistence_config() { Ok(cfg) => cfg, Err(err) => fatal_exit(format_args!("invalid persistence configuration: {err}")), @@ -3062,3 +3064,26 @@ fn fatal_exit(args: std::fmt::Arguments<'_>) -> ! { let _ = std::io::stderr().flush(); std::process::exit(1); } + +/// Route panics through `tracing::error!` so they show up in CI logs with +/// the same formatting as regular errors. Runs the default hook afterwards +/// so the process keeps its usual backtrace behaviour for developers +/// running locally with `RUST_BACKTRACE=1`. +fn install_panic_hook() { + let default = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + let location = info + .location() + .map(|l| format!("{}:{}:{}", l.file(), l.line(), l.column())) + .unwrap_or_else(|| "".to_string()); + let payload = info + .payload() + .downcast_ref::<&'static str>() + .copied() + .map(|s| s.to_string()) + .or_else(|| info.payload().downcast_ref::().cloned()) + .unwrap_or_else(|| "".to_string()); + tracing::error!(location = %location, payload = %payload, "panic"); + default(info); + })); +} diff --git a/crates/fakecloud-stepfunctions/src/interpreter.rs b/crates/fakecloud-stepfunctions/src/interpreter.rs index 5d8bc9b4c..97e3d3954 100644 --- a/crates/fakecloud-stepfunctions/src/interpreter.rs +++ b/crates/fakecloud-stepfunctions/src/interpreter.rs @@ -54,22 +54,55 @@ pub async fn execute_state_machine( }), ); - match run_states( - &def, - raw_input, - &delivery, - &dynamodb_state, - &state, - &execution_arn, - ) - .await - { - Ok(output) => { + // Run the state machine inside an inner tokio::spawn so that any panic + // bubbles up as a JoinError instead of tearing down the caller. Without + // this the panic propagates through the outer spawn in `start_execution` + // which leaves the execution stuck in Running and leaks the panic to + // tokio's default hook. + let def_owned = def; + let state_clone = state.clone(); + let execution_arn_clone = execution_arn.clone(); + let delivery_clone = delivery.clone(); + let dynamodb_state_clone = dynamodb_state.clone(); + let handle = tokio::spawn(async move { + run_states( + &def_owned, + raw_input, + &delivery_clone, + &dynamodb_state_clone, + &state_clone, + &execution_arn_clone, + ) + .await + }); + + match handle.await { + Ok(Ok(output)) => { succeed_execution(&state, &execution_arn, &output); } - Err((error, cause)) => { + Ok(Err((error, cause))) => { fail_execution(&state, &execution_arn, &error, &cause); } + Err(join_err) => { + let msg = if join_err.is_panic() { + let payload = join_err.into_panic(); + if let Some(s) = payload.downcast_ref::() { + s.clone() + } else if let Some(s) = payload.downcast_ref::<&'static str>() { + (*s).to_string() + } else { + "execution task panicked".to_string() + } + } else { + format!("execution task cancelled: {join_err}") + }; + tracing::error!( + execution_arn = %execution_arn, + panic = %msg, + "Step Functions execution panicked" + ); + fail_execution(&state, &execution_arn, "States.Runtime", &msg); + } } } diff --git a/crates/fakecloud-stepfunctions/src/service.rs b/crates/fakecloud-stepfunctions/src/service.rs index 560c01c81..d5d168ad9 100644 --- a/crates/fakecloud-stepfunctions/src/service.rs +++ b/crates/fakecloud-stepfunctions/src/service.rs @@ -804,15 +804,17 @@ fn validate_definition(definition: &str) -> Result<(), AwsServiceError> { )); } - let states = parsed.get("States").and_then(|v| v.as_object()); - if states.is_none() { - return Err(AwsServiceError::aws_error( - StatusCode::BAD_REQUEST, - "InvalidDefinition", - "Invalid State Machine Definition: 'MISSING_STATES' (States field is required)" - .to_string(), - )); - } + let states_obj = parsed + .get("States") + .and_then(|v| v.as_object()) + .ok_or_else(|| { + AwsServiceError::aws_error( + StatusCode::BAD_REQUEST, + "InvalidDefinition", + "Invalid State Machine Definition: 'MISSING_STATES' (States field is required)" + .to_string(), + ) + })?; let start_at = parsed["StartAt"].as_str().ok_or_else(|| { AwsServiceError::aws_error( @@ -822,7 +824,6 @@ fn validate_definition(definition: &str) -> Result<(), AwsServiceError> { .to_string(), ) })?; - let states_obj = states.unwrap(); if !states_obj.contains_key(start_at) { return Err(AwsServiceError::aws_error( StatusCode::BAD_REQUEST,