fix(wrappers/cron): don't leave an unhandled rejection when a bot promise rejects

[MrCooper42]

Problem

When a bot handler returns a promise, wrappers/cron.js attaches both promise.then(...) and promise.catch(...) to the same promise:

if (promise && typeof promise.then == "function" && botHandler.length < 3) {
    promise.then(data => { /* report complete */ });   // no rejection handler
}
if (promise && typeof promise.catch == "function") {
    promise.catch(err => { /* report error */ });       // attached to `promise`, not to the .then() chain
}

Those two are siblings, not a chain. When the handler's promise rejects, there are two outcomes:

1. The .catch() chain handles it correctly — logs [LEOCRON]:complete and reports status "error".
2. The .then()-derived chain also rejects, and nothing handles it.

Node's default unhandled-rejection mode is throw, and in that mode the rejection is first delivered to any unhandledRejection listeners before being raised as an exception. The Lambda runtime registers one — /var/runtime/index.mjs:1448, visible in the crash stack — which reports Runtime.UnhandledPromiseRejection and exits the process. The SDK never registers an unhandledRejection listener of its own, so the orphaned chain is fatal. (The wrapper's removal of uncaughtException listeners at lines 61-63 plays no role here; that path is never reached.)

On Lambda this surfaces as Runtime.UnhandledPromiseRejection → Runtime.ExitError, killing the sandbox mid-write — see below for what that costs.

Observed in production

A bot whose write stream exhausted its retries (leo-sdk's toLeo calls back with the bare string 'failed') produced this sequence — note the ordering, which shows leo's error branch starting and then the process being killed 5ms later:

20:28:35.238  ERROR  global "failed"                                       ← bot logged the rejection
20:28:35.239  INFO   [LEOCRON]:complete:…-media-switcher:0:…               ← the .catch() branch's log line
20:28:35.244  ERROR  Unhandled Promise Rejection
                     {"errorType":"Runtime.UnhandledPromiseRejection",
                      "errorMessage":"failed","reason":"failed", …}         ← the orphaned .then() chain
REPORT  Duration: 74747.77 ms  …  Status: error  Error Type: Runtime.ExitError

The error report never actually lands. The [LEOCRON]:complete line is only the log statement; the reportComplete DynamoDB write behind it is killed in flight when the process exits 5ms later, so the error status is never recorded and the cron lock is never released. CloudWatch shows the consequence: this run acquired its lock at 20:27:21.9 with a 300s Lambda timeout, and the next invocation arrived at 20:32:22.2 — exactly at lock expiry — where the normal cadence is back-to-back re-invocation within a second. (The preceding crash shows the same pattern to the second: lock at 20:22:20.5 → next run 20:27:21.9.) So each rejection today costs the real error report, ~4 minutes of the bot stalled on a dead lock, and a destroyed sandbox / cold start. This bot has been hitting it 1–6 times per day since at least July 26.

This is not specific to that 'failed' value. The botHandler.length < 3 gate means any botWrapper(async (settings, context) => …) bot gets the orphaned .then(), so any rejection from any async bot handler becomes a fatal Runtime.ExitError on top of the SDK's own correct error reporting.

Present on master and unchanged across at least 7.1.8 → 7.1.21.

Fix

Pass the rejection handler as the second argument to then() instead of attaching a separate .catch(). A rejection then has exactly one handler and no orphaned chain, while both existing branches keep their current behaviour:

• botHandler.length < 3 → success path plus error reporting.
• 3-arg (callback-style) handlers → still only the rejection path, since they report their own success through the callback.

Using then(onFulfilled, onRejected) rather than .then().catch() is deliberate: it keeps onRejected scoped to a rejection of the handler's promise, so a throw inside the success path isn't newly rerouted into the error branch. That keeps this change behaviour-preserving apart from removing the crash.

Behavioural note worth flagging for reviewers

Today a rejection attempts reportComplete("error") but the write is killed in flight (see above) — so in practice the failure is visible only in the Lambda Errors metric, while the LeoCron error record is lost and the lock stalls the bot until expiry. After this change the Lambda no longer fails — it calls callback(null, err), exactly as the existing callback-style path at line 176-181 already does, and the LeoCron error record reliably persists.

That is consistent with the SDK's design (bot errors are reported through the cron record, not through Lambda failure), but it does mean these failures stop appearing in the Lambda Errors metric and show up only via the LeoCron errorCount / botmon. Anyone alerting on Lambda errors for async bots would see those alarms go quiet — though as of 2026-08-08 neither affected function found so far (media-service-prod-media-switcher, item-prod-item-aux-update-catalog) has a CloudWatch alarm on it. I think consistency with the callback path is the right call, but it's a real monitoring change and I'd rather surface it than have it discovered later.

Testing

• npm run compile && mocha "test/*.utest.js" → 297 passing, 0 failures
• node --check wrappers/cron.js clean; npx eslint wrappers/cron.js clean

I did not add a unit test for this. wrappers/cron.js currently has no test coverage, and lib/mock-sdk.md states the position explicitly — "Don't test the wrapper — it's framework plumbing" — noting that the wrapper "creates its own internal SDK instance which cannot be swapped out." Testing it would need require-level interception of ../index.js, which isn't a pattern used anywhere in the suite today. Happy to add one if you'd like, and #212 looks like the natural home for that kind of harness.

Notes

• No Jira key on the branch, since I don't have access to the ES project — feel free to retitle if you want one attached.
• Separate, untouched observation: the err referenced inside the then() success handler resolves to the enclosing cron.checkLock callback's err (line 161), not the handler's error. It's always falsy on that path, so the current behaviour is correct, but it reads as though it were the handler's error. Left alone to keep this diff surgical.

---

Note

Medium Risk
Touches core cron/Lambda completion plumbing for all async bots; behavior change stops Lambda invocation failures on handler promise rejections, which can affect error-based alerting even though cron error reporting is unchanged.

Overview
Fixes a fatal Runtime.ExitError on async cron bots when the handler’s promise rejects.

Previously wrappers/cron.js attached separate promise.then() and promise.catch() on the same promise. On rejection, .catch() reported cron complete with "error", but the .then()-derived chain stayed unhandled and Node’s default unhandled-rejection behavior could kill the Lambda after the bot was already marked complete.

The change wires rejections through promise.then(onFulfilled, onRejected) (shared onRejected helper) for handlers with botHandler.length < 3, and uses only .catch(onRejected) for three-argument callback-style handlers. That preserves the existing success vs error reporting paths without an orphaned rejection chain.

After the fix, rejected async bots finish via callback(null, err) like the callback path instead of also failing the invocation—so Lambda Errors metrics may drop for these cases while cron/botmon error reporting remains.

^{Reviewed by Cursor Bugbot for commit b623277. Bugbot is set up for automated code reviews on this repo. Configure here.}

[ch-snyk-sa]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[emely-rithum]

Seeing the same signature on item-prod-item-aux-update-catalog (20:23 UTC), same stack through index.mjs:1448:17. Also getting two runs that time out at the 10-min hard limit instead of rejecting — @smithy/node-http-handler warnings about a 20s request hanging, then killed by Lambda with no unhandled-rejection logged. Not sure if this covers that hang case.

[jgrantr]

I independently re-verified this against the repo and CloudWatch before review. The diagnosis and the fix both hold up — the sibling .then()/.catch() orphaned-chain analysis is correct (confirmed with a standalone repro on Node 22), the quoted production event is real (7fa5fd22… on media-service-prod-media-switcher, 2026-08-06, exact timestamps and REPORT line), and it's ongoing at 1–6 crashes/day since at least July 26.

I edited the description to correct two details the logs contradicted (@MrCooper42, please yell if you'd rather I hadn't):

1. The error report doesn't actually land today. The [LEOCRON]:complete line is just the log statement — the reportComplete DynamoDB write is killed in flight when the runtime exits 5ms later. Evidence: the crashed run took its lock at 20:27:21.9 (300s Lambda timeout) and the next invocation arrived at 20:32:22.2, exactly at lock expiry, versus the normal sub-second back-to-back cadence. The prior crash shows the identical pattern (20:22:20.5 → 20:27:21.9). So the current behavior isn't "error reported twice" — it's "error report lost + bot stalled ~4 min per crash". Makes the fix strictly more valuable.

2. Mechanism nit: the crash stack shows the rejection is delivered to the Lambda runtime's own unhandledRejection listener (/var/runtime/index.mjs:1448), which posts the error and exits — the uncaughtException-listener removal at lines 61-63 isn't involved.

@emely-rithum — confirmed your item-prod-item-aux-update-catalog 20:23 UTC event is the same signature. The 10-minute hard-timeout runs are a different failure mode though: there the promise never settles (hung smithy request), so there's no rejection for this code path to observe. This PR can't and shouldn't cover that — the smithy hang deserves its own investigation.

On the monitoring note: neither affected function currently has a CloudWatch alarm on its Errors metric, so nothing goes quiet on merge that anyone is watching today.