Initiate connection from RFB constructor

We need to do this in order to safely attach to existing WebSocket
objects. There may be message events already pending so we must set up
our event handlers before returning.

This means we will now throw errors instead of generating "disconnect"
events on problems as the caller no longer has the opportunity to set up
event handlers.

This might have been the correct approach from the start as it mimics
how e.g. the WebSocket constructor works.

diff --git a/core/rfb.js b/core/rfb.js
index 074eb519a22f552ebff36421c9e7e7c5639dcbf3..228aeca426d18e5ded7412928f608b7539ae1235 100644 (file)
--- a/core/rfb.js
+++ b/core/rfb.js
@@ -239,9 +239,8 @@ export default class RFB extends EventTargetMixin {
         this._sock.on('message', this._handleMessage.bind(this));
         this._sock.on('error', this._socketError.bind(this));
 
-        // Slight delay of the actual connection so that the caller has
-        // time to set up callbacks
-        setTimeout(this._updateConnectionState.bind(this, 'connecting'));
+        // All prepared, kick off the connection
+        this._updateConnectionState('connecting');
 
         Log.Debug("<< RFB.constructor");
 
@@ -463,29 +462,20 @@ export default class RFB extends EventTargetMixin {
         Log.Debug(">> RFB.connect");
 
         if (this._url) {
-            try {
-                Log.Info(`connecting to ${this._url}`);
-                this._sock.open(this._url, this._wsProtocols);
-            } catch (e) {
-                if (e.name === 'SyntaxError') {
-                    this._fail("Invalid host or port (" + e + ")");
-                } else {
-                    this._fail("Error when opening socket (" + e + ")");
-                }
-            }
+            Log.Info(`connecting to ${this._url}`);
+            this._sock.open(this._url, this._wsProtocols);
         } else {
-            try {
-                Log.Info(`attaching ${this._rawChannel} to Websock`);
-                this._sock.attach(this._rawChannel);
-            } catch (e) {
-                this._fail("Error attaching channel (" + e + ")");
-            }
+            Log.Info(`attaching ${this._rawChannel} to Websock`);
+            this._sock.attach(this._rawChannel);
 
             if (this._sock.readyState === 'closed') {
-                this._fail("Cannot use already closed WebSocket/RTCDataChannel");
+                throw Error("Cannot use already closed WebSocket/RTCDataChannel");
             }
 
             if (this._sock.readyState === 'open') {
+                // FIXME: _socketOpen() can in theory call _fail(), which
+                //        isn't allowed this early, but I'm not sure that can
+                //        happen without a bug messing up our state variables
                 this._socketOpen();
             }
         }

diff --git a/tests/test.rfb.js b/tests/test.rfb.js
index c99edc754d7a1d34ff2b2611f33dfc621ff09790..404f7db1240bce4118c41d2c91c8919f74bee28d 100644 (file)
--- a/tests/test.rfb.js
+++ b/tests/test.rfb.js
@@ -151,34 +151,21 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 attach.restore();
             });
 
-            it('should not connect from constructor', function () {
-                new RFB(document.createElement('div'), 'wss://host:8675');
-                expect(open).to.not.have.been.called;
-                this.clock.tick(); // Flush the pending connection
-            });
-
             it('should actually connect to the websocket', function () {
                 new RFB(document.createElement('div'), 'ws://HOST:8675/PATH');
-                this.clock.tick();
                 expect(open).to.have.been.calledOnceWithExactly('ws://HOST:8675/PATH', []);
             });
 
-            it('should report connection problems via event', function () {
+            it('should pass on connection problems', function () {
                 open.restore();
                 open = sinon.stub(Websock.prototype, 'open');
-                open.throws(Error('Failure'));
-                const client = new RFB(document.createElement('div'), 'ws://HOST:8675/PATH');
-                let callback = sinon.spy();
-                client.addEventListener('disconnect', callback);
-                this.clock.tick();
-                expect(callback).to.have.been.calledOnce;
-                expect(callback.args[0][0].detail.clean).to.be.false;
+                open.throws(new Error('Failure'));
+                expect(() => new RFB(document.createElement('div'), 'ws://HOST:8675/PATH')).to.throw('Failure');
             });
 
             it('should handle WebSocket/RTCDataChannel objects', function () {
                 let sock = new FakeWebSocket('ws://HOST:8675/PATH', []);
                 new RFB(document.createElement('div'), sock);
-                this.clock.tick();
                 expect(open).to.not.have.been.called;
                 expect(attach).to.have.been.calledOnceWithExactly(sock);
             });
@@ -189,7 +176,6 @@ describe('Remote Frame Buffer Protocol Client', function () {
                 const client = new RFB(document.createElement('div'), sock);
                 let callback = sinon.spy();
                 client.addEventListener('disconnect', callback);
-                this.clock.tick();
                 expect(open).to.not.have.been.called;
                 expect(attach).to.have.been.calledOnceWithExactly(sock);
                 // Check if it is ready for some data
@@ -200,25 +186,15 @@ describe('Remote Frame Buffer Protocol Client', function () {
             it('should refuse closed WebSocket/RTCDataChannel objects', function () {
                 let sock = new FakeWebSocket('ws://HOST:8675/PATH', []);
                 sock.readyState = WebSocket.CLOSED;
-                const client = new RFB(document.createElement('div'), sock);
-                let callback = sinon.spy();
-                client.addEventListener('disconnect', callback);
-                this.clock.tick();
-                expect(callback).to.have.been.calledOnce;
-                expect(callback.args[0][0].detail.clean).to.be.false;
+                expect(() => new RFB(document.createElement('div'), sock)).to.throw();
             });
 
-            it('should report attach problems via event', function () {
+            it('should pass on attach problems', function () {
                 attach.restore();
                 attach = sinon.stub(Websock.prototype, 'attach');
-                attach.throws(Error('Failure'));
+                attach.throws(new Error('Failure'));
                 let sock = new FakeWebSocket('ws://HOST:8675/PATH', []);
-                const client = new RFB(document.createElement('div'), sock);
-                let callback = sinon.spy();
-                client.addEventListener('disconnect', callback);
-                this.clock.tick();
-                expect(callback).to.have.been.calledOnce;
-                expect(callback.args[0][0].detail.clean).to.be.false;
+                expect(() => new RFB(document.createElement('div'), sock)).to.throw('Failure');
             });
         });