Skip to content

fix(ext/node): don't emit ServerResponse 'finish' after client abort#34026

Merged
bartlomieju merged 1 commit into
mainfrom
orch/issue-47
May 14, 2026
Merged

fix(ext/node): don't emit ServerResponse 'finish' after client abort#34026
bartlomieju merged 1 commit into
mainfrom
orch/issue-47

Conversation

@lunadogbot

Copy link
Copy Markdown
Contributor

Summary

Fixes #34002.

When the client aborts an HTTP request before res.end() runs, the
server-side socket fires its 'close' event, which calls
emitCloseNT(res) on the in-flight ServerResponse. emitCloseNT sets
destroyed = true / _closed = true and emits 'close'. The deferred
res.end() then schedules onFinish via the _send callback,
and onFinish only checked socket._hadError — that flag is not set on
the abort path in Deno, so 'finish' was emitted after 'close'.

Node emits only 'close' in that scenario.

This PR adds a destroyed guard to onFinish so the response stops
emitting 'finish' once it has already been destroyed (either via the
response or its socket).

Reproducer (from #34002)

const http = require('http');

let client;
let count = 0;

const server = http.createServer((req, res) => {
  res.on('finish', () => { console.log('finish'); count++; });
  res.on('close', () => { console.log('close'); count++; });

  client.abort();

  setImmediate(() => {
    setImmediate(() => {
      res.end('ok');
      setImmediate(() => {
        server.close(() => { console.log('event count:', count); });
      });
    });
  });
});

server.listen(0, () => {
  client = http.get(`http://127.0.0.1:${server.address().port}`);
  client.on('error', () => {});
});

Before (Deno 2.7.14 release):

finish
close
event count: 2

After (Node, and Deno with this fix):

close
event count: 1

Test plan

  • Added tests/unit_node/http_test.ts regression test
    "[node/http] ServerResponse does not emit 'finish' after client abort"
  • All existing ServerResponse tests still pass (deno test -A tests/unit_node/http_test.ts --filter "ServerResponse")
  • tools/format.js clean
  • tools/lint.js --js clean

Closes bartlomieju/orchid-inbox#47

When the client aborts an HTTP request before `res.end()` runs, the
server-side socket fires its `'close'` event which calls `emitCloseNT`
on the in-flight `ServerResponse`, setting `destroyed = true`. The
deferred `res.end()` then schedules `onFinish` via the `_send` callback,
which only checked `socket._hadError` - that flag isn't set on the
abort path in Deno, so `'finish'` was still emitted after `'close'`.
Node only emits `'close'` in this scenario.

Guard `onFinish` against a destroyed response (or socket) so we match
Node's behavior.

Fixes #34002
@bartlomieju
bartlomieju merged commit 6a70eb7 into main May 14, 2026
136 checks passed
@bartlomieju
bartlomieju deleted the orch/issue-47 branch May 14, 2026 02:40
littledivy pushed a commit to crowlKats/deno that referenced this pull request Jun 10, 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.

http.ServerResponse emits both "finish" and "close" after aborted request

2 participants