Skip to content

fix(macros): reject instead of panicking when a queue or email handler returns Err - #1115

Open
PedroRosalba wants to merge 1 commit into
cloudflare:mainfrom
PedroRosalba:fix/queue-email-err-rejects
Open

PedroRosalba wants to merge 1 commit into
cloudflare:mainfrom
PedroRosalba:fix/queue-email-err-rejects

Conversation

@PedroRosalba

Copy link
Copy Markdown
Contributor

Problem

The queue and email glue in worker-macros/src/event.rs turn a handler Err into a panic:

Err(e) => {
    ::worker::console_log!("{}", &e);
    panic!("{}", e);
}

For a queue consumer, returning Err is the ordinary way to say "retry this batch". On the default build (panic=abort) the panic is a Wasm trap, though. It breaks every call in flight in the same isolate, and shim.js then reinitializes the instance on the next entry, so static state is lost.

Local repro (wrangler 4.151.2, std target, worker-build --release): a fetch awaits a 5 s Delay and then reads a static AtomicU64, and while it waits a batch is delivered to the consumer.

consumer in-flight fetch static after batch
Err(..) HTTP 500 (RuntimeError: null function, or "Cannot invoke closure from previous WASM instance" if it resumes after the reset) reset to 0 retried 1→2→3→DLQ
batch.retry_all(); Ok(()) 200 kept retried 1→2→3→DLQ
Ok(()) 200 kept acked

Fix

Return the error as a rejected promise, as the connect glue in the same file already does:

match #input_fn_ident(...).await {
    Ok(()) => Ok(::worker::wasm_bindgen::JsValue::UNDEFINED),
    Err(e) => Err(::worker::wasm_bindgen::JsValue::from(e)),
}

The runtime still retries the whole batch, as the docs describe ("If the queue() function throws, or the promise returned by it ... rejected, then the entire batch will be considered a failure and will be retried"), and it still counts as a failed invocation. The difference is that the instance survives. The email handler gets the same change.

Tests

Adds a queue test: the consumer returns Err the first time it sees a fail-once: message and remembers that in a static. The test checks that the batch is retried and succeeds on the second delivery (which only happens if the static survived), and that a message received before the Err is still in the existing GLOBAL_QUEUE_STATE.

Before the fix the new test fails (and the run logs the instance crashes); after it, the full suite passes. cargo fmt and clippy are clean. Email has no test harness here, so that change is covered by review only.

Not tested: the --panic-unwind build, where the panic should already be caught (worker/src/init.rs:39-42).

Related: #166 (panics killing later requests; a 2023 comment there hit it from a queue consumer), #1090 (errors under panic=unwind).

Tested on macOS (Apple Silicon) only.

…r returns Err

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

1 participant