http: emit 'drain' on OutgoingMessage only after buffers drain

lib/_http_outgoing.js

@@ -1139,12 +1139,12 @@ OutgoingMessage.prototype._flush = function _flush() {
 
   if (socket?.writable) {
     // There might be remaining data in this.output; write it out
-    const ret = this._flushOutput(socket);
+    this._flushOutput(socket);
 
     if (this.finished) {
       // This is a queue to the server or client to bring in the next this.
       this._finish();
-    } else if (ret && this[kNeedDrain]) {
+    } else if (this[kNeedDrain] && this.writableLength === 0) {
       this[kNeedDrain] = false;
       this.emit('drain');
     }

lib/_http_server.js

@@ -817,7 +817,13 @@ function socketOnDrain(socket, state) {
   }
 
   const msg = socket._httpMessage;
-  if (msg && !msg.finished && msg[kNeedDrain]) {
+  // Only emit 'drain' once the message has no data pending anywhere, so that
+  // msg.writableLength === 0 when the event fires. socketOnDrain is called
+  // synchronously from updateOutgoingData during _flushOutput, at which point
+  // the bytes we just handed to the socket (or the stale outputSize) mean
+  // the message is not actually drained yet - we wait for the socket's
+  // own 'drain' event instead.
+  if (msg && !msg.finished && msg[kNeedDrain] && msg.writableLength === 0) {
     msg[kNeedDrain] = false;
     msg.emit('drain');
   }

test/parallel/test-http-outgoing-drain-writable-length.js

@@ -0,0 +1,58 @@
+'use strict';
+// Regression test: when a pipelined ServerResponse (whose writes were
+// buffered in outputData while the socket belonged to a previous response)
+// is finally assigned its socket and flushed, 'drain' must not be emitted
+// until the socket's own buffer has actually drained. Previously,
+// socketOnDrain was called synchronously from _flushOutput via _onPendingData
+// and emitted 'drain' even though the bytes we just wrote were still sitting
+// in the socket's writable buffer, so res.writableLength was non-zero.
+
+const common = require('../common');
+const http = require('http');
+const net = require('net');
+const assert = require('assert');
+
+let step = 0;
+
+const server = http.createServer(common.mustCall((req, res) => {
+  step++;
+
+  if (step === 1) {
+    // Keep the first response open briefly so the second is queued with
+    // res.socket === null.
+    res.writeHead(200, { 'Content-Type': 'text/plain' });
+    setTimeout(() => res.end('ok'), 50);
+    return;
+  }
+
+  // Second (pipelined) response - queued in state.outgoing, no socket yet.
+  assert.strictEqual(res.socket, null);
+
+  res.writeHead(200, { 'Content-Type': 'text/plain' });
+
+  // Write past the response's highWaterMark so res.write() returns false
+  // and kNeedDrain is set. Data is buffered in outputData.
+  const chunk = Buffer.alloc(16 * 1024, 'x');
+  while (res.write(chunk));
+  assert.strictEqual(res.writableNeedDrain, true);
+
+  res.on('drain', common.mustCall(() => {
+    assert.strictEqual(
+      res.writableLength, 0,
+      `'drain' fired with writableLength=${res.writableLength}`,
+    );
+    res.end();
+    server.close();
+  }));
+}, 2));
+
+server.listen(0, common.mustCall(function() {
+  const port = this.address().port;
+  const client = net.connect(port);
+  client.write(
+    `GET /1 HTTP/1.1\r\nHost: localhost:${port}\r\n\r\n` +
+    `GET /2 HTTP/1.1\r\nHost: localhost:${port}\r\n\r\n`,
+  );
+  client.resume();
+  client.on('error', () => {});
+}));