Skip to content

Release sse request when stream setup throws - #3984

Closed
1cg wants to merge 1 commit into
four-devfrom
fix/sse-release-on-error
Closed

Release sse request when stream setup throws#3984
1cg wants to merge 1 commit into
four-devfrom
fix/sse-release-on-error

Conversation

@1cg

@1cg 1cg commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Follow-up to #3980.

Problem

#3980 changed the extension to hand core a manually resolved promise instead of the handleSSEResponse promise:

let releaseRequest;
ctx.extensionPromise = new Promise(resolve => releaseRequest = resolve);
handleSSEResponse(ctx, releaseRequest).catch(e => { ... });

release() is only reachable from the finally in handleSSEResponse, and that function does not open its try until line 181. Anything that throws between function entry and there skips the finally, and the catch does not release.

Core awaits extensionPromise at src/htmx.js:618, inside a finally, with __hideIndicators right after at line 625. So the await never returns. The indicator spins forever, the element stays in htmx-request, and no error surfaces to the user.

The window is real. getConfig(element) runs before release is even defined, so it hangs every mode, immediate included. api.htmxProp(element).sse = connection is also set before the try, and #3980 dropped the cleanup(element) that used to sit in the catch, so a throw there leaves the connection registered and blocks reconnects on that element.

Before #3980 this was safe. extensionPromise was the handleSSEResponse promise itself, so a rejection settled it and core continued.

Fix

Release and clean up in the catch:

handleSSEResponse(ctx, releaseRequest).catch(e => {
    releaseRequest();
    cleanup(element);
    ...

release() is already idempotent, so the normal path is unaffected.

Testing

Added releases the request when setup throws. It forces a throw in the setup window and asserts both symptoms: the indicator hides, and _htmx.sse does not stay registered.

Verified the test fails without the fix:

AssertionError: Indicator must hide when setup throws: expected true to be false

Full suite: 1732 passed, 0 failed, 100 percent coverage.

Draft, since #3980 only just landed and you may prefer to fold this in there.

@1cg
1cg force-pushed the fix/sse-release-on-error branch from 7e01cee to 60da0b8 Compare August 28, 2026 01:35
@MichaelWest22

Copy link
Copy Markdown
Collaborator

fine to make these changes. they will not trigger in a real situation but might be fine as defense in depth and to stop future work when the potential issue is discovered again later

@1cg 1cg closed this Aug 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants