Skip to content

Commit

Permalink
Revert "[protocol] Trigger onClose/onOpen whenever the underlying web…
Browse files Browse the repository at this point in the history
…socket OPEN/CLOSEs."

This reverts commit 05f8d75.
  • Loading branch information
geropl authored and roboquat committed Sep 29, 2021
1 parent 0065707 commit 2e87607
Show file tree
Hide file tree
Showing 3 changed files with 15 additions and 39 deletions.
28 changes: 9 additions & 19 deletions components/gitpod-protocol/src/messaging/browser/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@
*/

import { Logger, ConsoleLogger, toSocket, IWebSocket } from "vscode-ws-jsonrpc";
import { createMessageConnection } from "vscode-jsonrpc";
import { MessageConnection, createMessageConnection } from "vscode-jsonrpc";
import { AbstractMessageWriter } from "vscode-jsonrpc/lib/messageWriter";
import { AbstractMessageReader } from "vscode-jsonrpc/lib/messageReader";
import { JsonRpcProxyFactory, JsonRpcProxy } from "../proxy-factory";
import { ConnectionEventHandler, ConnectionHandler } from "../handler";
import { ConnectionHandler } from "../handler";
import ReconnectingWebSocket, { Event } from 'reconnecting-websocket';

export interface WebSocketOptions {
Expand All @@ -32,10 +32,7 @@ export class WebSocketConnectionProvider {
const startListening = (path: string) => {
const socket = this.listen({
path,
onConnection: c => factory.listen(c),
}, {
onTransportDidClose: () => factory.fireConnectionClosed(),
onTransportDidOpen: () => factory.fireConnectionOpened(),
onConnection: c => factory.listen(c)
},
options
);
Expand All @@ -55,7 +52,7 @@ export class WebSocketConnectionProvider {
/**
* Install a connection handler for the given path.
*/
listen(handler: ConnectionHandler, eventHandler: ConnectionEventHandler, options?: WebSocketOptions): WebSocket {
listen(handler: ConnectionHandler, options?: WebSocketOptions): WebSocket {
const url = handler.path;
const webSocket = this.createWebSocket(url);

Expand All @@ -72,8 +69,7 @@ export class WebSocketConnectionProvider {
}
doListen(
webSocket as any as ReconnectingWebSocket,
handler,
eventHandler,
connection => handler.onConnection(connection),
logger,
);
return webSocket;
Expand Down Expand Up @@ -104,22 +100,16 @@ export class WebSocketConnectionProvider {
// - webSocket.onopen: making sure it's only ever called once so we're re-using MessageConnection
// - WebSocketMessageWriter: buffer and re-try messages instead of throwing an error immidiately
// - WebSocketMessageReader: don't close MessageConnection on 'socket.onclose'
function doListen(resocket: ReconnectingWebSocket, handler: ConnectionHandler, eventHandler: ConnectionEventHandler, logger: Logger) {
resocket.addEventListener("close", () => eventHandler.onTransportDidClose());

function doListen(resocket: ReconnectingWebSocket, onConnection: (connection: MessageConnection) => void, logger: Logger) {
let alreadyOpened = false;
resocket.onopen = () => {
// trigerr "open" every time we re-open the underlying websocket
eventHandler.onTransportDidOpen();

// make sure we're only ever creating one MessageConnection, irregardless of how many times we have to re-open the underlying (reconnecting) websocket
if (alreadyOpened) {
return;
}
alreadyOpened = true;

const connection = createWebSocketConnection(resocket, logger);
handler.onConnection(connection);
onConnection(connection);
};
}

Expand Down Expand Up @@ -177,12 +167,12 @@ class BufferingWebSocketMessageWriter extends AbstractMessageWriter {
for (const msg of buffer) {
this.write(msg);
}
//this.logger.info(`flushed buffer (${this.buffer.length})`)
this.logger.info(`flushed buffer (${this.buffer.length})`)
}

protected bufferMsg(msg: any) {
this.buffer.push(msg);
//this.logger.info(`buffered message (${this.buffer.length})`);
this.logger.info(`buffered message (${this.buffer.length})`);
}
}

Expand Down
12 changes: 0 additions & 12 deletions components/gitpod-protocol/src/messaging/handler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,15 +13,3 @@ export interface ConnectionHandler {
readonly path: string;
onConnection(connection: MessageConnection, session?: object): void;
}

export interface ConnectionEventHandler {
/**
* Called when the transport underpinning the connection got closed
*/
onTransportDidClose(): void;

/**
* Called when the transport underpinning the connection is (re-)opened
*/
onTransportDidOpen(): void;
}
14 changes: 6 additions & 8 deletions components/gitpod-protocol/src/messaging/proxy-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,14 +104,12 @@ export class JsonRpcProxyFactory<T extends object> implements ProxyHandler<T> {
this.connectionPromise = new Promise(resolve =>
this.connectionPromiseResolve = resolve
);
}

fireConnectionClosed() {
this.onDidCloseConnectionEmitter.fire(undefined)
}

fireConnectionOpened() {
this.onDidOpenConnectionEmitter.fire(undefined);
this.connectionPromise.then(connection => {
connection.onClose(() =>
this.onDidCloseConnectionEmitter.fire(undefined)
);
this.onDidOpenConnectionEmitter.fire(undefined);
});
}

/**
Expand Down

0 comments on commit 2e87607

Please sign in to comment.