fix(node:http): set writableEnded to true when end() called without socket

[majiayu000]

Summary

• ServerResponse.end() now correctly sets finished=true when called without a valid socket/handle
• This ensures writableEnded returns true after end() is called, per Node.js spec

Test plan

• Added regression test in test/regression/issue/25632.test.ts
• All existing HTTP tests pass

Fixes #25632

[coderabbitai]

Walkthrough

ServerResponse.end() was changed to mark the response finished when no underlying handle exists (setting finished/writableEnded), and a regression test was added to verify writableEnded/finished becomes true across several end() scenarios.

Changes

Cohort / File(s)	Change Summary
Implementation fix src/js/node/_http_server.ts	Adjusted ServerResponse.prototype.end control flow to set this.finished = true when the internal socket/handle is absent; removed invoking the callback as a substitute for marking finished.
Regression tests test/regression/issue/25632.test.ts	Added tests covering: end() with no socket, end() with callback but no socket, end() with string chunk but no socket, and end() in a real HTTP server — asserting writableEnded/finished are true after end().

Suggested reviewers

• Jarred-Sumner

Pre-merge checks

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing writableEnded behavior when end() is called without a socket.
Description check	✅ Passed	The description covers the key aspects: what was fixed (finished=true setting), how it was verified (regression test and existing tests pass), and the issue reference.
Linked Issues check	✅ Passed	The PR successfully addresses issue #25632 by ensuring ServerResponse.end() sets finished=true (making writableEnded true) even without a socket, matching Node.js spec behavior.
Out of Scope Changes check	✅ Passed	All changes are directly related to fixing the writableEnded behavior issue: the ServerResponse.end() fix and the regression test for issue #25632.

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 2247c38 and b00ee11.

📒 Files selected for processing (2)

• src/js/node/_http_server.ts
• test/regression/issue/25632.test.ts

🧰 Additional context used

📓 Path-based instructions (7)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

• src/js/node/_http_server.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

• src/js/node/_http_server.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

• test/regression/issue/25632.test.ts

test/regression/issue/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

Regression tests for specific issues go in /test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory

Files:

• test/regression/issue/25632.test.ts

**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in tests - use Bun's Glob and built-in tools instead

Files:

• test/regression/issue/25632.test.ts

test/regression/issue/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Place regression tests for specific GitHub issues in test/regression/issue/${issueNumber}.test.ts with real issue numbers only

Files:

• test/regression/issue/25632.test.ts

test/**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Always use port: 0 in tests - do not hardcode ports or use custom random port number functions

Files:

• test/regression/issue/25632.test.ts

🧠 Learnings (13)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22756
File: src/js/node/_http_server.ts:945-947
Timestamp: 2025-09-19T21:17:04.690Z
Learning: JSNodeHTTPServerSocket in src/bun.js/bindings/NodeHTTP.cpp implements both end() and close() methods. The end() method properly handles socket termination for both HTTP responses and raw CONNECT sockets by setting ended=true, checking buffered data, and flushing remaining data through the response context.

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-88
Timestamp: 2025-10-19T04:55:27.213Z
Learning: In the Bun.telemetry._node_binding API for Node.js http.createServer compatibility, there are exactly two hooks: handleIncomingRequest(req, res) called once per request (returns a request ID), and handleWriteHead(res, statusCode) called once when headers are sent (explicit or implicit). The old handleRequestFinish hook no longer exists after architectural refactoring. Both hooks are tested in test/js/bun/http/node-telemetry.test.ts.

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

📚 Learning: 2025-09-19T21:17:04.690Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22756
File: src/js/node/_http_server.ts:945-947
Timestamp: 2025-09-19T21:17:04.690Z
Learning: JSNodeHTTPServerSocket in src/bun.js/bindings/NodeHTTP.cpp implements both end() and close() methods. The end() method properly handles socket termination for both HTTP responses and raw CONNECT sockets by setting ended=true, checking buffered data, and flushing remaining data through the response context.

Applied to files:

• src/js/node/_http_server.ts
• test/regression/issue/25632.test.ts

📚 Learning: 2025-10-19T04:55:27.213Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-88
Timestamp: 2025-10-19T04:55:27.213Z
Learning: In the Bun.telemetry._node_binding API for Node.js http.createServer compatibility, there are exactly two hooks: handleIncomingRequest(req, res) called once per request (returns a request ID), and handleWriteHead(res, statusCode) called once when headers are sent (explicit or implicit). The old handleRequestFinish hook no longer exists after architectural refactoring. Both hooks are tested in test/js/bun/http/node-telemetry.test.ts.

Applied to files:

• src/js/node/_http_server.ts

📚 Learning: 2025-10-19T04:55:33.099Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Applied to files:

• src/js/node/_http_server.ts
• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-10-18T05:23:24.403Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Track resources (servers, clients) in arrays for cleanup in `afterEach()`

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

• test/regression/issue/25632.test.ts

🧬 Code graph analysis (1)

test/regression/issue/25632.test.ts (1)

test/js/node/test/parallel/test-http-outgoing-proto.js (1)

• ServerResponse (8-8)

🔇 Additional comments (4)

src/js/node/_http_server.ts (1)

1335-1336: LGTM! Correct fix for writableEnded behavior.

The fix correctly sets finished = true when end() is called without a valid handle, ensuring writableEnded returns true as per Node.js spec. This makes the behavior consistent with the code path at line 1377 where finished is also set to true when a handle exists.

test/regression/issue/25632.test.ts (3)

11-24: LGTM! Comprehensive test for the fix.

This test directly validates that writableEnded and finished become true after calling end() without a valid socket, which is the core issue being fixed.

---

26-41: LGTM! Validates callback behavior.

This test ensures that the callback handling via process.nextTick still works correctly after the fix, and that writableEnded becomes true immediately when end() is called.

---

43-51: LGTM! Covers edge case with data chunk.

This test validates that passing data to end() doesn't prevent writableEnded from becoming true when there's no socket.

[Jarred-Sumner]

If we rewrite the test using node:test and try running it with node, the test should be true after end() is called with callback but without socket fails in node.

/**
 * Regression test for issue #25632
 * ServerResponse.end() should always result in writableEnded being set to/returning true
 *
 * @see https://github.com/oven-sh/bun/issues/25632
 */
import assert from "node:assert";
import http, { createServer, ServerResponse } from "node:http";
import { describe, test } from "node:test";

describe("ServerResponse.writableEnded", () => {
  test("should be true after end() is called without a socket", async () => {
    // Create a ServerResponse without a valid socket/handle
    const req = new http.IncomingMessage(null as any);
    const res = new ServerResponse(req);

    assert.strictEqual(res.writableEnded, false);
    assert.strictEqual(res.finished, false);

    res.end();

    // Per Node.js spec, writableEnded should be true after end() is called
    assert.strictEqual(res.writableEnded, true);
    assert.strictEqual(res.finished, true);
  });

  test("should be true after end() is called with callback but without socket", async () => {
    const req = new http.IncomingMessage(null as any);
    const res = new ServerResponse(req);

    let callbackCalled = false;
    res.end(() => {
      callbackCalled = true;
    });

    assert.strictEqual(res.writableEnded, true);
    assert.strictEqual(res.finished, true);

    // Wait for callback to be called via nextTick
    await new Promise(resolve => process.nextTick(resolve));
    assert.strictEqual(callbackCalled, true);
  });

  test("should be true after end() with chunk but without socket", async () => {
    const req = new http.IncomingMessage(null as any);
    const res = new ServerResponse(req);

    res.end("test data");

    assert.strictEqual(res.writableEnded, true);
    assert.strictEqual(res.finished, true);
  });

  test("should be true in normal server context", async () => {
    const server = createServer((req, res) => {
      assert.strictEqual(res.writableEnded, false);
      res.end("Hello");
      assert.strictEqual(res.writableEnded, true);
    });

    server.listen({ port: 0 });
    const { port } = server.address() as { port: number };

    try {
      const response = await fetch(`http://localhost:${port}`);
      assert.strictEqual(await response.text(), "Hello");
    } finally {
      server.close();
    }
  });
});

[coderabbitai]

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between b00ee11 and 36f48fd.

📒 Files selected for processing (2)

• src/js/node/_http_server.ts
• test/regression/issue/25632.test.ts

🧰 Additional context used

📓 Path-based instructions (7)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

src/js/{builtins,node,bun,thirdparty,internal}/**/*.{ts,js}: Use .$call() and .$apply() instead of .call() and .apply() to prevent user tampering with function invocation
Use string literal require() statements only; dynamic requires are not permitted
Export modules using export default { ... } syntax; modules are NOT ES modules
Use JSC intrinsics (prefixed with $) such as $Array.from(), $isCallable(), and $newArrayWithSize() for performance-critical operations
Use private globals and methods with $ prefix (e.g., $Array, map.$set()) instead of public JavaScript globals
Use $debug() for debug logging and $assert() for assertions; both are stripped in release builds
Validate function arguments using validators from internal/validators and throw $ERR_* error codes for invalid arguments
Use process.platform and process.arch for platform detection; these values are inlined and dead-code eliminated at build time

Files:

• src/js/node/_http_server.ts

src/js/{builtins,node,bun,thirdparty,internal}/**/*.ts

📄 CodeRabbit inference engine (src/js/CLAUDE.md)

Builtin functions must include this parameter typing in TypeScript to enable direct method binding in C++

Files:

• src/js/node/_http_server.ts

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}

📄 CodeRabbit inference engine (test/CLAUDE.md)

test/**/*.test.{ts,js,jsx,tsx,mjs,cjs}: Use bun:test with files that end in *.test.{ts,js,jsx,tsx,mjs,cjs}
Do not write flaky tests. Never wait for time to pass in tests; always wait for the condition to be met instead of using an arbitrary amount of time
Never use hardcoded port numbers in tests. Always use port: 0 to get a random port
Prefer concurrent tests over sequential tests using test.concurrent or describe.concurrent when multiple tests spawn processes or write files, unless it's very difficult to make them concurrent
When spawning Bun processes in tests, use bunExe and bunEnv from harness to ensure the same build of Bun is used and debug logging is silenced
Use -e flag for single-file tests when spawning Bun processes
Use tempDir() from harness to create temporary directories with files for multi-file tests instead of creating files manually
Prefer async/await over callbacks in tests
When callbacks must be used and it's just a single callback, use Promise.withResolvers to create a promise that can be resolved or rejected from a callback
Do not set a timeout on tests. Bun already has timeouts
Use Buffer.alloc(count, fill).toString() instead of 'A'.repeat(count) to create repetitive strings in tests, as ''.repeat is very slow in debug JavaScriptCore builds
Use describe blocks for grouping related tests
Always use await using or using to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc
Always check exit codes and test error scenarios in error tests
Use describe.each() for parameterized tests
Use toMatchSnapshot() for snapshot testing
Use beforeAll(), afterEach(), beforeEach() for setup/teardown in tests
Track resources (servers, clients) in arrays for cleanup in afterEach()

Files:

• test/regression/issue/25632.test.ts

test/regression/issue/**/*.test.ts

📄 CodeRabbit inference engine (test/CLAUDE.md)

Regression tests for specific issues go in /test/regression/issue/${issueNumber}.test.ts. Do not put tests without issue numbers in the regression directory

Files:

• test/regression/issue/25632.test.ts

**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

**/*.test.ts?(x): Never use bun test directly - always use bun bd test to run tests with debug build changes
For single-file tests, prefer -e flag over tempDir
For multi-file tests, prefer tempDir and Bun.spawn over single-file tests
Use normalizeBunSnapshot to normalize snapshot output of tests
Never write tests that check for 'panic', 'uncaught exception', or similar strings in test output
Use tempDir from harness to create temporary directories - do not use tmpdirSync or fs.mkdtempSync
When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure
Do not write flaky tests - do not use setTimeout in tests; instead await the condition to be met
Verify tests fail with USE_SYSTEM_BUN=1 bun test <file> and pass with bun bd test <file> - tests are invalid if they pass with USE_SYSTEM_BUN=1
Test files must end with .test.ts or .test.tsx
Avoid shell commands like find or grep in tests - use Bun's Glob and built-in tools instead

Files:

• test/regression/issue/25632.test.ts

test/regression/issue/*.test.ts

📄 CodeRabbit inference engine (CLAUDE.md)

Place regression tests for specific GitHub issues in test/regression/issue/${issueNumber}.test.ts with real issue numbers only

Files:

• test/regression/issue/25632.test.ts

test/**/*.test.ts?(x)

📄 CodeRabbit inference engine (CLAUDE.md)

Always use port: 0 in tests - do not hardcode ports or use custom random port number functions

Files:

• test/regression/issue/25632.test.ts

🧠 Learnings (19)

📓 Common learnings

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22756
File: src/js/node/_http_server.ts:945-947
Timestamp: 2025-09-19T21:17:04.690Z
Learning: JSNodeHTTPServerSocket in src/bun.js/bindings/NodeHTTP.cpp implements both end() and close() methods. The end() method properly handles socket termination for both HTTP responses and raw CONNECT sockets by setting ended=true, checking buffered data, and flushing remaining data through the response context.

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-88
Timestamp: 2025-10-19T04:55:27.213Z
Learning: In the Bun.telemetry._node_binding API for Node.js http.createServer compatibility, there are exactly two hooks: handleIncomingRequest(req, res) called once per request (returns a request ID), and handleWriteHead(res, statusCode) called once when headers are sent (explicit or implicit). The old handleRequestFinish hook no longer exists after architectural refactoring. Both hooks are tested in test/js/bun/http/node-telemetry.test.ts.

📚 Learning: 2025-09-19T21:17:04.690Z

Learnt from: cirospaciari
Repo: oven-sh/bun PR: 22756
File: src/js/node/_http_server.ts:945-947
Timestamp: 2025-09-19T21:17:04.690Z
Learning: JSNodeHTTPServerSocket in src/bun.js/bindings/NodeHTTP.cpp implements both end() and close() methods. The end() method properly handles socket termination for both HTTP responses and raw CONNECT sockets by setting ended=true, checking buffered data, and flushing remaining data through the response context.

Applied to files:

• src/js/node/_http_server.ts
• test/regression/issue/25632.test.ts

📚 Learning: 2025-10-19T04:55:33.099Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-203
Timestamp: 2025-10-19T04:55:33.099Z
Learning: In test/js/bun/http/node-telemetry.test.ts and the Bun.telemetry._node_binding API, after the architecture refactor, the _node_binding interface only contains two methods: handleIncomingRequest(req, res) and handleWriteHead(res, statusCode). The handleRequestFinish hook and other lifecycle hooks were removed during simplification. Both current methods are fully tested.

Applied to files:

• src/js/node/_http_server.ts
• test/regression/issue/25632.test.ts

📚 Learning: 2025-10-19T04:55:27.213Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/http/node-telemetry.test.ts:27-88
Timestamp: 2025-10-19T04:55:27.213Z
Learning: In the Bun.telemetry._node_binding API for Node.js http.createServer compatibility, there are exactly two hooks: handleIncomingRequest(req, res) called once per request (returns a request ID), and handleWriteHead(res, statusCode) called once when headers are sent (explicit or implicit). The old handleRequestFinish hook no longer exists after architectural refactoring. Both hooks are tested in test/js/bun/http/node-telemetry.test.ts.

Applied to files:

• src/js/node/_http_server.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/regression/issue/**/*.test.ts : Regression tests for specific issues go in `/test/regression/issue/${issueNumber}.test.ts`. Do not put tests without issue numbers in the regression directory

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/regression/issue/*.test.ts : Place regression tests for specific GitHub issues in `test/regression/issue/${issueNumber}.test.ts` with real issue numbers only

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:36:59.706Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: src/bun.js/bindings/v8/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:36:59.706Z
Learning: Applies to src/bun.js/bindings/v8/test/v8/v8.test.ts : Add corresponding test cases to test/v8/v8.test.ts using checkSameOutput() function to compare Node.js and Bun output

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always check exit codes and test error scenarios in error tests

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to **/*.test.ts?(x) : When spawning processes in tests, expect stdout before expecting exit code for more useful error messages on test failure

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Track resources (servers, clients) in arrays for cleanup in `afterEach()`

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-10-18T05:23:24.403Z

Learnt from: theshadow27
Repo: oven-sh/bun PR: 23798
File: test/js/bun/telemetry-server.test.ts:91-100
Timestamp: 2025-10-18T05:23:24.403Z
Learning: In the Bun codebase, telemetry tests (test/js/bun/telemetry-*.test.ts) should focus on telemetry API behavior: configure/disable/isEnabled, callback signatures and invocation, request ID correlation, and error handling. HTTP protocol behaviors like status code normalization (e.g., 200 with empty body → 204) should be tested in HTTP server tests (test/js/bun/http/), not in telemetry tests. Keep separation of concerns: telemetry tests verify the telemetry API contract; HTTP tests verify HTTP semantics.

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*-fixture.ts : Test files that spawn Bun processes should end in `*-fixture.ts` to identify them as test fixtures and not tests themselves

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Always use `await using` or `using` to ensure proper resource cleanup in tests for APIs like Bun.listen, Bun.connect, Bun.spawn, Bun.serve, etc

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Never use hardcoded port numbers in tests. Always use `port: 0` to get a random port

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-12-16T00:21:32.179Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-16T00:21:32.179Z
Learning: Applies to test/**/*.test.ts?(x) : Always use `port: 0` in tests - do not hardcode ports or use custom random port number functions

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-09-03T01:30:58.001Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 21728
File: test/js/valkey/valkey.test.ts:264-271
Timestamp: 2025-09-03T01:30:58.001Z
Learning: For test/js/valkey/valkey.test.ts PUB/SUB tests, avoid arbitrary sleeps and async-forEach. Instead, resolve a Promise from the subscriber callback when the expected number of messages is observed and await it with a bounded timeout (e.g., withTimeout + Promise.withResolvers) to account for Redis server→subscriber propagation.

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-06T00:58:23.965Z

Learnt from: markovejnovic
Repo: oven-sh/bun PR: 24417
File: test/js/bun/spawn/spawn.test.ts:903-918
Timestamp: 2025-11-06T00:58:23.965Z
Learning: In Bun test files, `await using` with spawn() is appropriate for long-running processes that need guaranteed cleanup on scope exit or when explicitly testing disposal behavior. For short-lived processes that exit naturally (e.g., console.log scripts), the pattern `const proc = spawn(...); await proc.exited;` is standard and more common, as evidenced by 24 instances vs 4 `await using` instances in test/js/bun/spawn/spawn.test.ts.

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-09-02T05:33:37.517Z

Learnt from: Jarred-Sumner
Repo: oven-sh/bun PR: 22323
File: test/js/web/websocket/websocket-subprotocol.test.ts:74-75
Timestamp: 2025-09-02T05:33:37.517Z
Learning: In Bun's runtime, `await using` with Node.js APIs like `net.createServer()` is properly supported and should not be replaced with explicit cleanup. Bun has extended Node.js APIs with proper async dispose support.

Applied to files:

• test/regression/issue/25632.test.ts

📚 Learning: 2025-11-24T18:37:30.259Z

Learnt from: CR
Repo: oven-sh/bun PR: 0
File: test/CLAUDE.md:0-0
Timestamp: 2025-11-24T18:37:30.259Z
Learning: Applies to test/**/*.test.{ts,js,jsx,tsx,mjs,cjs} : Prefer async/await over callbacks in tests

Applied to files:

• test/regression/issue/25632.test.ts

🧬 Code graph analysis (1)

test/regression/issue/25632.test.ts (1)

test/js/node/test/parallel/test-http-outgoing-proto.js (1)

• ServerResponse (8-8)

🔇 Additional comments (4)

test/regression/issue/25632.test.ts (3)

11-24: LGTM!

The test correctly verifies that writableEnded and finished return true after end() is called without a socket, which is the core fix for issue #25632.

---

40-48: LGTM!

The test correctly verifies that writableEnded and finished are true after end() is called with a data chunk but without a socket.

---

50-66: LGTM!

The test validates normal server operation and properly cleans up resources. The server cleanup is correctly awaited (line 64), which addresses the past review comment and ensures proper resource management as per coding guidelines.

src/js/node/_http_server.ts (1)

1334-1340: The no-handle behavior correctly matches Node.js.

The implementation properly handles the case where there is no socket:

• Sets finished = true to make writableEnded return true
• Does not invoke the callback (verified Node.js does the same)
• Does not emit events (verified Node.js emits nothing: prefinish, finish, and close are not triggered)

The code and comments accurately document the intended behavior.

[coderabbitai]

🧹 Nitpick | 🔵 Trivial

Consider explicitly testing that callback is not invoked.

The test passes a callback to end() and documents that it should not be invoked (lines 31-32), but doesn't actually verify this behavior. While the test correctly checks writableEnded and finished, explicitly testing callback non-invocation would make the test more robust and self-documenting.

🔎 Optional enhancement to verify callback behavior

  test("should be true after end() is called with callback but without socket", async () => {
    const req = new http.IncomingMessage(null as any);
    const res = new ServerResponse(req);
+   let callbackInvoked = false;

    res.end(() => {
-     // Note: In Node.js, callback is NOT invoked when there's no socket
-     // This matches Node.js behavior where the 'finish' event never fires without a socket
+     callbackInvoked = true;
    });

    // Per Node.js spec, writableEnded should be true after end() is called
    expect(res.writableEnded).toBe(true);
    expect(res.finished).toBe(true);
+   
+   // Wait one tick to ensure callback would have been invoked if it were going to be
+   await new Promise(resolve => process.nextTick(resolve));
+   expect(callbackInvoked).toBe(false); // Callback should NOT be invoked without socket
  });

🤖 Prompt for AI Agents

In test/regression/issue/25632.test.ts around lines 26 to 38, the test documents
that the callback passed to res.end() should not be invoked but never asserts
it; add an explicit assertion by replacing the empty callback with a spy/flag
(e.g., a jest.fn() or a boolean set inside the callback) when calling
res.end(...) then assert that the spy/flag was not called/changed after end()
and after any necessary microtask ticks, keeping the existing expectations for
writableEnded and finished.