make request timeout errors eligible for retry (#1104)

stripe · Jan 15, 2021 · e89cc76 · e89cc76
1 parent 337ff6c
commit e89cc76
Show file tree

Hide file tree

Showing 3 changed files with 83 additions and 17 deletions.
diff --git a/lib/StripeResource.js b/lib/StripeResource.js
@@ -98,17 +98,7 @@ StripeResource.prototype = {
       const timeoutErr = new TypeError('ETIMEDOUT');
       timeoutErr.code = 'ETIMEDOUT';
 
-      req._isAborted = true;
-      req.abort();
-
-      callback.call(
-        this,
-        new StripeConnectionError({
-          message: `Request aborted due to timeout being reached (${timeout}ms)`,
-          detail: timeoutErr,
-        }),
-        null
-      );
+      req.destroy(timeoutErr);
     };
   },
 
@@ -224,11 +214,7 @@ StripeResource.prototype = {
   },
 
   _errorHandler(req, requestRetries, callback) {
-    return (error) => {
-      if (req._isAborted) {
-        // already handled
-        return;
-      }
+    return (message, detail) => {
       callback.call(
         this,
         new StripeConnectionError({
@@ -486,7 +472,23 @@ StripeResource.prototype = {
             null
           );
         } else {
-          return this._errorHandler(req, requestRetries, callback)(error);
+          if (error.code === 'ETIMEDOUT') {
+            return callback.call(
+              this,
+              new StripeConnectionError({
+                message: `Request aborted due to timeout being reached (${timeout}ms)`,
+                detail: error,
+              })
+            );
+          }
+          return callback.call(
+            this,
+            new StripeConnectionError({
+              message: this._generateConnectionErrorMessage(requestRetries),
+              detail: error,
+            }),
+            null
+          );
         }
       });
 

diff --git a/test/StripeResource.spec.js b/test/StripeResource.spec.js
@@ -185,6 +185,46 @@ describe('StripeResource', () => {
         });
       });
 
+      it('throws an error on connection timeout', (done) => {
+        return utils.getTestServerStripe(
+          {timeout: 10},
+          (req, res) => {
+            // Do nothing. This will trigger a timeout.
+          },
+          (err, stripe) => {
+            if (err) {
+              return done(err);
+            }
+            stripe.charges.create(options.data, (err, result) => {
+              expect(err.detail.message).to.deep.equal('ETIMEDOUT');
+              done();
+            });
+          }
+        );
+      });
+
+      it('retries connection timeout errors', (done) => {
+        let nRequestsReceived = 0;
+        return utils.getTestServerStripe(
+          {timeout: 10, maxNetworkRetries: 2},
+          (req, res) => {
+            nRequestsReceived += 1;
+            // Do nothing. This will trigger a timeout.
+            return {shouldStayOpen: nRequestsReceived < 3};
+          },
+          (err, stripe) => {
+            if (err) {
+              return done(err);
+            }
+            stripe.charges.create(options.data, (err, result) => {
+              expect(err.detail.message).to.deep.equal('ETIMEDOUT');
+              expect(nRequestsReceived).to.equal(3);
+              done();
+            });
+          }
+        );
+      });
+
       it('should retry the request if max retries are set', (done) => {
         nock(`https://${options.host}`)
           .post(options.path, options.params)

diff --git a/testUtils/index.js b/testUtils/index.js
@@ -6,9 +6,33 @@ require('mocha');
 // Ensure we are using the 'as promised' libs before any tests are run:
 require('chai').use(require('chai-as-promised'));
 
+const http = require('http');
+
 const ResourceNamespace = require('../lib/ResourceNamespace').ResourceNamespace;
 
 const utils = (module.exports = {
+  getTestServerStripe: (clientOptions, handler, callback) => {
+    const server = http.createServer((req, res) => {
+      const {shouldStayOpen} = handler(req, res) || {};
+      if (!shouldStayOpen) {
+        res.on('close', () => server.close());
+      }
+    });
+    server.listen(0, () => {
+      const {port} = server.address();
+      const stripe = require('../lib/stripe')(
+        module.exports.getUserStripeKey(),
+        {
+          host: 'localhost',
+          port,
+          protocol: 'http',
+          ...clientOptions,
+        }
+      );
+      return callback(null, stripe);
+    });
+  },
+
   getUserStripeKey: () => {
     const key =
       process.env.STRIPE_TEST_API_KEY || 'tGN0bIwXnHdwOa85VABjPdSn8nWY7G7I';