Skip to content
Closed
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 28 additions & 2 deletions src/js/node/_http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,13 @@ const MathFloor = Math.floor;

let cluster;

// diagnostics_channel channels for the HTTP server. Mirrors Node's
// lib/_http_server.js. Inactive channels are no-ops until someone subscribes.
const dc = require("node:diagnostics_channel");
const onRequestStartChannel = dc.channel("http.server.request.start");
const onResponseCreatedChannel = dc.channel("http.server.response.created");
const onResponseFinishChannel = dc.channel("http.server.response.finish");

function emitCloseServer(self: Server) {
callCloseCallback(self);
self.emit("close");
Expand Down Expand Up @@ -661,7 +668,7 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort
if (!requestShouldKeepAlive(http_req)) {
http_res[kMustCloseConnection] = true;
}
http_res.once("finish", endSocketOnFinishIfNeeded.bind(undefined, socket, http_res));
http_res.once("finish", resOnFinish.bind(undefined, http_req, http_res, socket, server));

if (hasObserver("http")) {
startPerf(http_res, kServerResponseStatistics, {
Expand Down Expand Up @@ -758,6 +765,11 @@ Server.prototype[kRealListen] = function (tls, port, host, socketPath, reusePort
http_req._dumpAndCloseReadable();
}

// Match Node's parserOnIncoming: publish once, before branching, for
// non-upgrade requests (fires on 503/checkContinue/417/normal paths).
if (!is_upgrade && onRequestStartChannel.hasSubscribers) {
onRequestStartChannel.publish({ request: http_req, response: http_res, socket, server });
}
Comment thread
claude[bot] marked this conversation as resolved.
if (reachedRequestsLimit) {
server.emit("dropRequest", http_req, socket);
http_res.writeHead(503);
Expand Down Expand Up @@ -1519,6 +1531,12 @@ function ServerResponse(req, options): void {
this.statusCode = 200;
this.statusMessage = undefined;
this.chunkedEncoding = false;

// Publish response.created from the constructor (matches Node) so it also
// fires for direct `new ServerResponse(req)` use — light-my-request etc.
if (onResponseCreatedChannel.hasSubscribers) {
onResponseCreatedChannel.publish({ request: req, response: this });
}
Comment thread
robobun marked this conversation as resolved.
}
$toClass(ServerResponse, "ServerResponse", OutgoingMessage);

Expand Down Expand Up @@ -1684,7 +1702,15 @@ function stopServerResponsePerf(this: any) {
}
}

function endSocketOnFinishIfNeeded(socket, res) {
function resOnFinish(req, res, socket, server) {
if (onResponseFinishChannel.hasSubscribers) {
onResponseFinishChannel.publish({
request: req,
response: res,
socket,
server,
});
}
if (res[kMustCloseConnection]) {
socket?.end();
}
Expand Down
18 changes: 18 additions & 0 deletions src/js/node/http2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,13 @@
const onServerStreamErrorChannel = dc.channel("http2.server.stream.error");
const onServerStreamFinishChannel = dc.channel("http2.server.stream.finish");
const onServerStreamCloseChannel = dc.channel("http2.server.stream.close");
// node:_http_server's HTTP server channels, for the allowHTTP1 fallback path.
// response.created is published by the ServerResponse constructor; the other
// two are published in connectionListenerHTTP1 so subscribers see the same
// three events Node fires on this path. Same channel objects as
// node:_http_server (keyed by name in diagnostics_channel's registry).
const onHttp1RequestStartChannel = dc.channel("http.server.request.start");
const onHttp1ResponseFinishChannel = dc.channel("http.server.response.finish");
const { Readable } = Stream;
type Http2ConnectOptions = {
settings?: Settings;
Expand Down Expand Up @@ -5562,16 +5569,27 @@
};

const res = new ServerResponseClass(req);
// Stable reference for the diagnostics closure: the outer `req` is reused
// across pipelined requests on this connection.
const request = req;
const handle = createHttp1FallbackResponseHandle(socket, shouldKeepAlive, keepAliveTimeout);
handle.onfinished = function () {
socket[kHttp1ActiveRequests] = Math.max(0, (socket[kHttp1ActiveRequests] || 1) - 1);
// Publish before socket.end() so subscribers observe the socket before
// its writable side is ended, like resOnFinish in node:_http_server.
if (onHttp1ResponseFinishChannel.hasSubscribers) {
onHttp1ResponseFinishChannel.publish({ request, response: res, socket, server });
}

Check warning on line 5582 in src/js/node/http2.ts

View check run for this annotation

Claude / Claude Code Review

http2 allowHTTP1: response.finish now publishes before res.finished flips true

Moving the allowHTTP1 `response.finish` publish into `handle.onfinished` fixed `socket.writableEnded` but traded it for the mirror-image divergence: `handle.onfinished` runs synchronously inside `handle.end()` — before `ServerResponse.prototype.end` sets `this.finished = true` and before `emit('finish')` — so on this path a subscriber now observes `payload.response.finished === false` / `writableFinished === false`, whereas the plain `http.createServer` path (and Node, where `resOnFinish` is a `
Comment thread
claude[bot] marked this conversation as resolved.
if (!shouldKeepAlive && !socket.destroyed) {
socket.end();
}
};
res[kHttp1ResponseHandle] = handle;
res.assignSocket(socket);

if (onHttp1RequestStartChannel.hasSubscribers) {
onHttp1RequestStartChannel.publish({ request, response: res, socket, server });
}
server.emit("request", req, res);
return 0;
};
Expand Down
Loading
Loading