Fix/hx-live test unawaited promise - #3902
Conversation
The test wrapped its hx-live body in a self-invoking async IIFE. hx-live bodies run with expression=false, so the body statements execute directly as an AsyncFunction body (top-level await already works) -- the IIFE's own promise was never awaited or caught by anything. debounce()'s cancellation mechanism rejects the superseded promise, and with no one holding a reference to the IIFE's promise, that rejection went fully unhandled, which crashes strict hosts (e.g. deno_core) on unhandled rejection. Write the body as plain statements instead of a self-invoking IIFE.
|
Sorry about the target branch confusion: I corrected it to point to |
|
Thanks for tracking this down. The test fix looks good. I pushed a small docs follow-up to your branch. It moves the warning into Notes on both the |
|
Great. Thanks! |
window.addEventListener('unhandledrejection', e => {
if (e.reason === dbSym) e.preventDefault();
});maybe we should add this in to catch this unhandled rejection. We have to use this symbol just to handle canceling the debounce which is the only way to abort it in JS. It is not a real error just using it as the only escape hatch we have. So ideally we should be catching and ignoring this non error. I don't know if the documentation we added is 100% accurate as while top level async is now the best way to handle things the wrapped versions still work fine most of the time and it is only this one debounce unhandled exception issue that the wrapped form highlights but other uses while more code don't cause issues. |
|
I didn't give it deep though. Feel free to completely replace my added note from docs. |
Description
test/tests/ext/hx-live.js's'debounce(ms) supersedes prior calls'test writes itshx-livebody as a self-invoking async IIFE:hx-live="..."bodies run withexpression=false(__executeJavaScript,htmx.js:920):With
expression=false,codebecomes the statement body of the generatedAsyncFunction— it's executed, not returned.(async () => {...})()creates a promise that nothing in the call chain ever holds a reference to: it's a bare expression statement, and its value is discarded.hx-live.js'srun()wrapsawait api.executeJavaScript(...)intry/catch, but that only covers a rejection of the outer wrapper — which resolves immediately, since its own body never awaits the inner IIFE. The inner IIFE's promise is fully orphaned.That orphaned promise is exactly the one
debounce()'s cancellation mechanism rejects (makeDebounce(),hx-live.js:354-376, rejects the superseded call with a private sentinel). Every real call site that awaits a promise-formdebounce()does so directly inside its owntry/catch, so this rejection is normally caught and silently discriminated viaif (e !== dbSym) console.error(...). This test is the one place a promise-formdebounce()ends up not directly awaited by anything, because of the redundant IIFE — so its rejection goes fully unhandled.In a browser this is cosmetic (an
Uncaught (in promise) Symbol()console warning), but it's a genuinely unhandled rejection per spec — any host that treats unhandled rejections as fatal aborts the entire in-flight evaluation when this fires.Why this is specific to
expression=false, not IIFE-wrapping in general: if the same body ran underexpression=true(e.g. an:attr="..."binding), the code is compiled asreturn (${code}). The IIFE's promise becomes the outerAsyncFunction's return value, and per spec, returning a thenable from an async function causes the runtime to implicitly await it before settling the outer function's own promise. So underexpression=truethe same IIFE pattern would not orphan the rejection — it would propagate correctly to the caller'stry/catch. The bug is inherent to the statement-body execution mode thathx-live="..."uses, not to unawaited IIFEs generally.The fix:
hx-livebodies executed withexpression=falsealready run as the body of anAsyncFunction, so top-levelawaitworks directly — this is already documented (www/src/content/extensions/15-hx-live.md:164: "await works at the top level (expressions areasyncfunctions)"). The IIFE was unnecessary; writing the body as plain statements fixes it:With that change, the test's
debounce()promise is awaited directly byrun()'s owntry/catch, so the cancellation rejection is caught and discriminated viadbSymexactly like every other call site.Also added a one-line callout to the docs (
15-hx-live.md): "Don't leave a promise unawaited — htmx won't see it, and its errors will be swallowed silently."Testing
Ran
npm run test:chromelocally before and after the change: 162 passed, 0 failed, 1 skipped, both before and after — the fix doesn't alter test behavior in a standard browser, since the bug's effect there is silent (console-only).The bug becomes fatal outside a browser: verified against a downstream embedder (a V8-via-
deno_coreruntime) that treats unhandled promise rejections as fatal — there, the unfixed test aborts the entire test-file evaluation; with the one-line fix, it passes cleanly.When running the tests in the browser, this appears in the console:
Suggestion for catching regressions like this in CI (not implemented in this PR): because this failure mode is silent in a real browser,
npm run testwould not catch a reintroduction of this pattern. Awindow.addEventListener('unhandledrejection', ...)check could be added to fail a test if it leaves any unhandled rejection behind. Note if this is picked up later: a naive implementation via a sharedbeforeEach/afterEachhook has a real pitfall — Mocha fails all remaining tests in the suite when a hook throws (confirmed: a hook-level failure here cascaded to 143 failing tests, versus 1 for an equivalent plain assertion failure). I didn't implement any of this, since it affects the test flow in a major way and needs pondering.Checklist
masterfor website changes,devfor source changes)npm run test) and verified that it succeeded