From cfcb8fdad8f55276504801ce52298e8fed9792d1 Mon Sep 17 00:00:00 2001 From: Aaron Gowatch Date: Fri, 20 Dec 2019 11:18:38 -0700 Subject: [PATCH] Improve reconnect callback fidelity --- .../ReconnectingPromisedWebSocket.ts | 6 ++++-- .../DefaultScreenSharingSession.ts | 13 +++++++----- .../session/DefaultScreenViewingSession.ts | 8 ++++++-- .../ReconnectingPromisedWebSocket.test.ts | 20 +++++++++++++++++++ .../PromisedWebSocketMock.ts | 10 ++++++++-- .../DefaultScreenSharingSession.test.ts | 17 ++++------------ 6 files changed, 50 insertions(+), 24 deletions(-) diff --git a/src/promisedwebsocket/ReconnectingPromisedWebSocket.ts b/src/promisedwebsocket/ReconnectingPromisedWebSocket.ts index 414e580ad3..1cf0a1c82d 100644 --- a/src/promisedwebsocket/ReconnectingPromisedWebSocket.ts +++ b/src/promisedwebsocket/ReconnectingPromisedWebSocket.ts @@ -65,10 +65,12 @@ export default class ReconnectingPromisedWebSocket implements PromisedWebSocket this.dispatchEvent(event); }); - this.webSocket.open(timeoutMs).then((event: Event) => { + this.webSocket.addEventListener('open', (event: Event) => { this.didOpenWebSocket(); - resolve(event); + this.dispatchEvent(event); }); + + return this.webSocket.open(timeoutMs).then(event => resolve(event)); }); } diff --git a/src/screensharingsession/DefaultScreenSharingSession.ts b/src/screensharingsession/DefaultScreenSharingSession.ts index bb4080c61b..5e3b4ac998 100644 --- a/src/screensharingsession/DefaultScreenSharingSession.ts +++ b/src/screensharingsession/DefaultScreenSharingSession.ts @@ -39,6 +39,7 @@ export default class DefaultScreenSharingSession implements ScreenSharingSession }); this.webSocket.addEventListener('close', (event: CloseEvent) => { + this.logger.warn('screen sharing connection closed'); this.stop().catch(() => {}); this.observerQueue.forEach((observer: ScreenSharingSessionObserver) => { Maybe.of(observer.didClose).map(f => f.bind(observer)(event)); @@ -46,21 +47,23 @@ export default class DefaultScreenSharingSession implements ScreenSharingSession }); this.webSocket.addEventListener('reconnect', (event: Event) => { - this.logger.warn('WebSocket reconnecting'); + this.logger.warn('screen sharing connection reconnecting'); this.stop().catch(() => {}); this.observerQueue.forEach((observer: ScreenSharingSessionObserver) => { Maybe.of(observer.willReconnect).map(f => f.bind(observer)(event)); }); }); - this.logger.info(`opening screen sharing connection to ${this.webSocket.url}`); - - return this.webSocket.open(timeoutMs).then((event: Event) => { + this.webSocket.addEventListener('open', (event: Event) => { + this.logger.warn('screen sharing connection opened'); this.observerQueue.forEach((observer: ScreenSharingSessionObserver) => { Maybe.of(observer.didOpen).map(f => f.bind(observer)(event)); }); - return event; }); + + this.logger.info(`opening screen sharing connection to ${this.webSocket.url}`); + + return this.webSocket.open(timeoutMs); } close(timeoutMs: number): Promise { diff --git a/src/screenviewing/session/DefaultScreenViewingSession.ts b/src/screenviewing/session/DefaultScreenViewingSession.ts index 99cf5b28d0..7e8413304c 100644 --- a/src/screenviewing/session/DefaultScreenViewingSession.ts +++ b/src/screenviewing/session/DefaultScreenViewingSession.ts @@ -38,11 +38,15 @@ export default class DefaultScreenViewingSession implements ScreenViewingSession ); this.webSocket.addEventListener('message', (event: MessageEvent) => { - Maybe.of(this.observer.didReceiveWebSocketMessage).map(f => f.bind(this.observer)(event)); + Maybe.of(this.observer).map(observer => { + Maybe.of(observer.didReceiveWebSocketMessage).map(f => f.bind(this.observer)(event)); + }); }); this.webSocket.addEventListener('close', (event: CloseEvent) => { - Maybe.of(this.observer.didCloseWebSocket).map(f => f.bind(this.observer)(event)); + Maybe.of(this.observer).map(observer => { + Maybe.of(observer.didCloseWebSocket).map(f => f.bind(this.observer)(event)); + }); }); return this.webSocket.open(request.timeoutMs); diff --git a/test/promisedwebsocket/ReconnectingPromisedWebSocket.test.ts b/test/promisedwebsocket/ReconnectingPromisedWebSocket.test.ts index 01b400f3bd..8bd45fb2fb 100644 --- a/test/promisedwebsocket/ReconnectingPromisedWebSocket.test.ts +++ b/test/promisedwebsocket/ReconnectingPromisedWebSocket.test.ts @@ -36,6 +36,26 @@ describe('ReconnectingPromisedWebSocket', () => { }); describe('#open', () => { + describe('with error', () => { + it('is rejected', (done: Mocha.Done) => { + const webSocket = { + ...Substitute.for(), + open(_timeoutMs: number): Promise { + throw new Error(); + }, + }; + const webSocketFactory = Substitute.for(); + const subject = new ReconnectingPromisedWebSocket( + url, + protocols, + binaryType, + webSocketFactory, + backoff + ); + webSocketFactory.create(Arg.all()).returns(webSocket); + chai.expect(subject.open(timeoutMs)).to.eventually.be.rejected.and.notify(done); + }); + }); describe('without open', () => { it('is fulfilled', (done: Mocha.Done) => { const webSocketFactory = Substitute.for(); diff --git a/test/promisedwebsocketmock/PromisedWebSocketMock.ts b/test/promisedwebsocketmock/PromisedWebSocketMock.ts index 78a1fbfd2e..98e9e9d350 100644 --- a/test/promisedwebsocketmock/PromisedWebSocketMock.ts +++ b/test/promisedwebsocketmock/PromisedWebSocketMock.ts @@ -14,11 +14,17 @@ export default class PromisedWebSocketMock implements PromisedWebSocket { } open(_timeoutMs: number): Promise { - return Promise.resolve(Substitute.for()); + const event = Substitute.for(); + event.type.returns('open'); + this.dispatchEvent(event); + return Promise.resolve(event); } close(_timeoutMs: number): Promise { - return Promise.resolve(Substitute.for()); + const event = Substitute.for(); + event.type.returns('close'); + this.dispatchEvent(event); + return Promise.resolve(event); } send(_data: string | ArrayBufferLike | Blob | ArrayBufferView): Promise { diff --git a/test/screensharingsession/DefaultScreenSharingSession.test.ts b/test/screensharingsession/DefaultScreenSharingSession.test.ts index ee9f8c5366..b22d848a83 100644 --- a/test/screensharingsession/DefaultScreenSharingSession.test.ts +++ b/test/screensharingsession/DefaultScreenSharingSession.test.ts @@ -39,7 +39,7 @@ describe('DefaultScreenSharingSession', function() { describe('#open', () => { describe('with open', () => { it('is fulfilled', (done: Mocha.Done) => { - const promisedWebSocket = Substitute.for(); + const promisedWebSocket = new PromisedWebSocketMock(); const subject = new DefaultScreenSharingSession( promisedWebSocket, constraintsProvider, @@ -50,17 +50,12 @@ describe('DefaultScreenSharingSession', function() { mediaRecordingFactory, logging ); - const event = Substitute.for(); - event.type.returns('open'); - promisedWebSocket.open(Arg.any()).returns(Promise.resolve(event)); - promisedWebSocket.url.returns(''); subject.open(1000).should.eventually.be.fulfilled.and.notify(done); - promisedWebSocket.dispatchEvent(event); }); }); - it('nofifies', (done: Mocha.Done) => { - const promisedWebSocket = Substitute.for(); + it('notifies', (done: Mocha.Done) => { + const promisedWebSocket = new PromisedWebSocketMock(); const subject = new DefaultScreenSharingSession( promisedWebSocket, constraintsProvider, @@ -71,15 +66,11 @@ describe('DefaultScreenSharingSession', function() { mediaRecordingFactory, logging ); - const event = Substitute.for(); const observer: ScreenSharingSessionObserver = { - didOpen(event: Event): void { - chai.expect(event).to.eq(event); + didOpen(_event: Event): void { done(); }, }; - promisedWebSocket.open(Arg.any()).returns(Promise.resolve(event)); - promisedWebSocket.url.returns(''); subject.registerObserver(observer); subject.open(1000).should.eventually.be.fulfilled; });