Merge pull request #577 from darkdarkdragon/develop-reconnect-2

remove Request resubmit logic
XRPLF · Oct 5, 2015 · 634fe56 · 634fe56
2 parents 9ebb595 + 225ca3f
commit 634fe56
Show file tree

Hide file tree

Showing 3 changed files with 65 additions and 45 deletions.
diff --git a/src/core/request.js b/src/core/request.js
@@ -34,14 +34,15 @@ function Request(remote, command) {
     command: command,
     id: undefined
   };
+  this._timeout = this.remote.submission_timeout;
 }
 
 util.inherits(Request, EventEmitter);
 
 // Send the request to a remote.
 Request.prototype.request = function(servers, callback_) {
-  const self = this;
   const callback = typeof servers === 'function' ? servers : callback_;
+  const self = this;
 
   if (this.requested) {
     throw new Error('Already requested');
@@ -70,17 +71,25 @@ Request.prototype.request = function(servers, callback_) {
     }
   }
 
-  function onReconnect() {
-    doRequest();
-  }
+  const timeout = setTimeout(() => {
+    if (typeof callback === 'function') {
+      callback(new RippleError('tejTimeout'));
+    }
+
+    this.emit('timeout');
+    // just in case
+    this.emit = _.noop;
+    this.cancel();
+  }, this._timeout);
 
   function onResponse() {
-    self.remote.removeListener('connected', onReconnect);
+    clearTimeout(timeout);
   }
 
   if (this.remote.isConnected()) {
-    this.remote.on('connected', onReconnect);
+    this.remote.on('connected', doRequest);
   }
+
   this.once('response', onResponse);
 
   doRequest();
@@ -264,38 +273,11 @@ Request.prototype.callback = function(callback, successEvent, errorEvent) {
   return this;
 };
 
-Request.prototype.timeout = function(duration, callback) {
-  const self = this;
-
-  function requested() {
-    self.timeout(duration, callback);
-  }
-
-  if (!this.requested) {
-    // Defer until requested
-    return this.once('request', requested);
+Request.prototype.setTimeout = function(delay) {
+  if (!_.isFinite(delay)) {
+    throw new Error('delay must be number');
   }
-
-  const emit = this.emit;
-  let timed_out = false;
-
-  const timeout = setTimeout(function() {
-    timed_out = true;
-
-    if (typeof callback === 'function') {
-      callback();
-    }
-
-    emit.call(self, 'timeout');
-    self.cancel();
-  }, duration);
-
-  this.emit = function() {
-    if (!timed_out) {
-      clearTimeout(timeout);
-      emit.apply(self, arguments);
-    }
-  };
+  this._timeout = delay;
 
   return this;
 };

diff --git a/src/core/transactionmanager.js b/src/core/transactionmanager.js
@@ -698,7 +698,8 @@ TransactionManager.prototype._request = function(tx) {
 
   tx.emit('postsubmit');
 
-  submitRequest.timeout(self._submissionTimeout, requestTimeout);
+  submitRequest.setTimeout(self._submissionTimeout);
+  submitRequest.once('timeout', requestTimeout);
 };
 
 /**

diff --git a/test/request-test.js b/test/request-test.js
@@ -65,6 +65,38 @@ describe('Request', function() {
 
   });
 
+  it('Send request - reconnect', function(done) {
+    const server = makeServer('wss://localhost:5006');
+    let emitted = 0;
+
+    const remote = new Remote();
+    remote._connected = true;
+    remote._servers = [server];
+
+    server._request = function(req) {
+      assert(req instanceof Request);
+      assert.strictEqual(typeof req.message, 'object');
+      assert.strictEqual(req.message.command, 'server_info');
+      if (++emitted === 1) {
+        setTimeout(function() {
+          remote.emit('connected');
+        }, 2);
+      } if (emitted === 2) {
+        setTimeout(function() {
+          req.emit('success', SERVER_INFO);
+          req.emit('response', SERVER_INFO);
+        }, 2);
+      }
+    };
+
+    const request = new Request(remote, 'server_info');
+
+    request.callback(function() {
+      assert.strictEqual(emitted, 2);
+      done();
+    });
+  });
+
   it('Send request -- filterRequest', function(done) {
     const servers = [
       makeServer('wss://localhost:5006'),
@@ -538,6 +570,7 @@ describe('Request', function() {
       setTimeout(function() {
         successEmitted = true;
         req.emit('success', SERVER_INFO);
+        req.emit('response', SERVER_INFO);
       }, 200);
     };
 
@@ -546,8 +579,9 @@ describe('Request', function() {
     remote._servers = [server];
 
     const request = new Request(remote, 'server_info');
+    request.setTimeout(10);
 
-    request.timeout(10, function() {
+    request.on('timeout', function() {
       setTimeout(function() {
         assert(successEmitted);
         done();
@@ -568,7 +602,8 @@ describe('Request', function() {
       assert.strictEqual(req.message.command, 'server_info');
       setTimeout(function() {
         req.emit('success', SERVER_INFO);
-      }, 200);
+        req.emit('response', SERVER_INFO);
+      }, 20);
     };
 
     const remote = new Remote();
@@ -583,13 +618,15 @@ describe('Request', function() {
       timedOut = true;
     });
 
-    request.timeout(1000);
+    request.setTimeout(100);
 
     request.callback(function(err, res) {
-      assert(!timedOut);
-      assert.ifError(err);
-      assert.deepEqual(res, SERVER_INFO);
-      done();
+      setTimeout(function() {
+        assert(!timedOut, 'must not timeout');
+        assert.ifError(err);
+        assert.deepEqual(res, SERVER_INFO);
+        done();
+      }, 100);
     });
   });