Skip to content

fix(eio-client): abort the pending polling requests once the close packet is sent - #5630

Open
mark1russell7 wants to merge 1 commit into
socketio:mainfrom
mark1russell7:fix/eio-client-abort-polling-requests
Open

mark1russell7 wants to merge 1 commit into
socketio:mainfrom
mark1russell7:fix/eio-client-abort-polling-requests

Conversation

@mark1russell7

Copy link
Copy Markdown

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behavior

When the polling transport is closed, it sends a close packet, but the requests that are still pending (the long-polling GET request, and a POST request in flight) are not aborted. If the server, or a proxy, does not answer them, they stay open after the close event.

New behavior

The close packet is still sent first, so that the server sees a clean close. Once the close request is done (or failed), the transport calls a new abortRequests() method:

  • Fetch: each request (except the close request, which is done at this point) uses the signal of one AbortController of the transport, which is aborted.
  • XHR: BaseXHR keeps the pending requests in a set, and aborts them with Request#abort(true) (_cleanup(true) calls xhr.abort()). Request#abort() without the argument still only detaches the request, as before (it is used by the unload handler).
  • xmlhttprequest-ssl (Node.js): abortRequests() does nothing, because abort() on a pending request throws an uncaught TypeError from version 2.1.2 (Abort causes TypeError: Cannot read properties of null (reading 'reusedSocket') mjwwit/node-XMLHttpRequest#37).

The requests are not aborted before the close request is done on purpose: the Engine.IO server answers the pending GET request when it receives the close packet, and before it answers the close request. If the GET request is aborted at once instead, the server can see it closed before the close packet, and then it closes the session with the reason transport error instead of transport close.

doWrite() gets an optional third argument, called when the request fails, so that the transport knows when the close request is done.

Other information (e.g. related issues)

Fixes #5628

The new test in test/node.js ("should abort the long-polling request once the close packet is sent") uses a small HTTP server that answers the handshake and the POST requests, but never answers the long-polling request. When the server receives the long-polling request, it calls socket.close(), and the test expects the server to see the long-polling request closed. It runs with USE_FETCH=1, and is skipped with xmlhttprequest-ssl (see above).

Test results, from one run on a GitHub Actions runner (Linux, Node.js 24.22.2), at the commit 7155a8a of this branch on top of main (f97d51a). The job follows ci-engine.io-client.yml: npm ci, then it compiles engine.io-parser, engine.io-client and engine.io. "Before" is the lib/ folder of main with the tests of this branch.

npx mocha --exit --require test/support/hooks.js --grep long-polling test/node.js
before after
default (xmlhttprequest-ssl) 0 passing, 1 pending 0 passing, 1 pending
USE_FETCH=1 0 passing, 1 failing 1 passing

Full suites of the branch: test:node 109 passing, 2 pending. test:node-fetch 109 passing, 2 pending. test:node-builtin-ws 106 passing, 5 pending. format:check passes.

The run: https://github.com/mark1russell7/wirelab/actions/runs/43625189318

This PR was prepared with AI assistance (Claude). The tests named above ran before and after the change in that run, with the results above.

🤖 Generated with Claude Code

…cket is sent

When the polling transport was closed, it sent a "close" packet, but it
did not abort the requests which were still pending, like the
long-polling request. If the server (or a proxy) never answered it, the
request stayed open after the "close" event, holding a connection (and,
in Node.js, preventing the process from exiting).

The pending requests are now aborted once the close packet is sent (or
cannot be sent). They are not aborted before, so that the server still
sees a clean close.

Note: with the "xmlhttprequest-ssl" package (Node.js), the requests are
not aborted, because aborting a pending request throws an uncaught
exception (see mjwwit/node-XMLHttpRequest#37).

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[engine.io-client] polling transport: the long-polling request stays open after the socket is closed

1 participant