From b9ffca1a003eb4320e3541e637a071e2f6d9279c Mon Sep 17 00:00:00 2001
From: themez <themezeng@gmail.com>
Date: Thu, 26 Sep 2019 11:56:59 +0800
Subject: [PATCH] http: add reusedSocket property on client request

Set ClientRequest.reusedSocket property when reusing socket for request,
so user can handle retry base on wether the request is reusing a socket.

Refs: https://github.com/request/request/issues/3131

PR-URL: https://github.com/nodejs/node/pull/29715
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
---
 doc/api/http.md                            | 56 ++++++++++++++++++++++
 lib/_http_agent.js                         |  1 +
 lib/_http_client.js                        |  1 +
 test/parallel/test-http-agent-keepalive.js |  9 ++--
 4 files changed, 64 insertions(+), 3 deletions(-)

diff --git a/doc/api/http.md b/doc/api/http.md
index cfe26a812927e7..2f6b27e10c652e 100644
--- a/doc/api/http.md
+++ b/doc/api/http.md
@@ -673,6 +673,62 @@ Removes a header that's already defined into headers object.
 request.removeHeader('Content-Type');
 ```
 
+### `request.reusedSocket`
+
+<!-- YAML
+added: REPLACEME
+-->
+
+* {boolean} Whether the request is send through a reused socket.
+
+When sending request through a keep-alive enabled agent, the underlying socket
+might be reused. But if server closes connection at unfortunate time, client
+may run into a 'ECONNRESET' error.
+
+```js
+const http = require('http');
+
+// Server has a 5 seconds keep-alive timeout by default
+http
+  .createServer((req, res) => {
+    res.write('hello\n');
+    res.end();
+  })
+  .listen(3000);
+
+setInterval(() => {
+  // Adapting a keep-alive agent
+  http.get('http://localhost:3000', { agent }, (res) => {
+    res.on('data', (data) => {
+      // Do nothing
+    });
+  });
+}, 5000); // Sending request on 5s interval so it's easy to hit idle timeout
+```
+
+By marking a request whether it reused socket or not, we can do
+automatic error retry base on it.
+
+```js
+const http = require('http');
+const agent = new http.Agent({ keepAlive: true });
+
+function retriableRequest() {
+  const req = http
+    .get('http://localhost:3000', { agent }, (res) => {
+      // ...
+    })
+    .on('error', (err) => {
+      // Check if retry is needed
+      if (req.reusedSocket && err.code === 'ECONNRESET') {
+        retriableRequest();
+      }
+    });
+}
+
+retriableRequest();
+```
+
 ### `request.setHeader(name, value)`
 <!-- YAML
 added: v1.6.0
diff --git a/lib/_http_agent.js b/lib/_http_agent.js
index 0fbf25abab35fd..3bc2d0e6abeb1a 100644
--- a/lib/_http_agent.js
+++ b/lib/_http_agent.js
@@ -340,6 +340,7 @@ Agent.prototype.keepSocketAlive = function keepSocketAlive(socket) {
 
 Agent.prototype.reuseSocket = function reuseSocket(socket, req) {
   debug('have free socket');
+  req.reusedSocket = true;
   socket.ref();
 };
 
diff --git a/lib/_http_client.js b/lib/_http_client.js
index 0e2e3b7c671f4d..fb3adbbcdd591a 100644
--- a/lib/_http_client.js
+++ b/lib/_http_client.js
@@ -204,6 +204,7 @@ function ClientRequest(input, options, cb) {
   this.upgradeOrConnect = false;
   this.parser = null;
   this.maxHeadersCount = null;
+  this.reusedSocket = false;
 
   let called = false;
 
diff --git a/test/parallel/test-http-agent-keepalive.js b/test/parallel/test-http-agent-keepalive.js
index 8cfc568b1e76e4..0a85b30b0a42ce 100644
--- a/test/parallel/test-http-agent-keepalive.js
+++ b/test/parallel/test-http-agent-keepalive.js
@@ -63,7 +63,8 @@ function checkDataAndSockets(body) {
 
 function second() {
   // Request second, use the same socket
-  get('/second', common.mustCall((res) => {
+  const req = get('/second', common.mustCall((res) => {
+    assert.strictEqual(req.reusedSocket, true);
     assert.strictEqual(res.statusCode, 200);
     res.on('data', checkDataAndSockets);
     res.on('end', common.mustCall(() => {
@@ -80,7 +81,8 @@ function second() {
 
 function remoteClose() {
   // Mock remote server close the socket
-  get('/remote_close', common.mustCall((res) => {
+  const req = get('/remote_close', common.mustCall((res) => {
+    assert.deepStrictEqual(req.reusedSocket, true);
     assert.deepStrictEqual(res.statusCode, 200);
     res.on('data', checkDataAndSockets);
     res.on('end', common.mustCall(() => {
@@ -120,7 +122,8 @@ function remoteError() {
 server.listen(0, common.mustCall(() => {
   name = `localhost:${server.address().port}:`;
   // Request first, and keep alive
-  get('/first', common.mustCall((res) => {
+  const req = get('/first', common.mustCall((res) => {
+    assert.strictEqual(req.reusedSocket, false);
     assert.strictEqual(res.statusCode, 200);
     res.on('data', checkDataAndSockets);
     res.on('end', common.mustCall(() => {