Support HTTP long polling as an alternative to WebSockets

app/client/components/GristClientSocket.ts

@@ -0,0 +1,152 @@
+import WS from 'ws';
+import { Socket as EIOSocket } from 'engine.io-client';
+
+export interface GristClientSocketOptions {
+  headers?: Record<string, string>;
+}
+
+export class GristClientSocket {
+  // Exactly one of _wsSocket and _eioSocket will be set at any one time.
+  private _wsSocket: WS.WebSocket | WebSocket | undefined;
+  private _eioSocket: EIOSocket | undefined;
+
+  // Set to true if a WebSocket connection (in _wsSocket) was succesfully
+  // established. Errors from the underlying WebSocket are not forwarded to
+  // the client until that point, in case we end up downgrading to Engine.IO.
+  private _wsConnected: boolean = false;
+
+  private _messageHandler: null | ((data: string) => void);
+  private _openHandler: null | (() => void);
+  private _errorHandler: null | ((err: Error) => void);
+  private _closeHandler: null | (() => void);
+
+  constructor(private _url: string, private _options?: GristClientSocketOptions) {
+    this._createWSSocket();
+  }
+
+  public set onmessage(cb: null | ((data: string) => void)) {
+    this._messageHandler = cb;
+  }
+
+  public set onopen(cb: null | (() => void)) {
+    this._openHandler = cb;
+  }
+
+  public set onerror(cb: null | ((err: Error) => void)) {
+    this._errorHandler = cb;
+  }
+
+  public set onclose(cb: null | (() => void)) {
+    this._closeHandler = cb;
+  }
+
+  public close() {
+    if (this._wsSocket) {
+      this._wsSocket.close();
+    } else {
+      this._eioSocket!.close();
+    }
+  }
+
+  public send(data: string) {
+    if (this._wsSocket) {
+      this._wsSocket.send(data);
+    } else {
+      this._eioSocket!.send(data);
+    }
+  }
+
+  // pause() and resume() are used for tests and assume a WS.WebSocket transport
+  public pause() {
+    (this._wsSocket as WS.WebSocket)?.pause();
+  }
+
+  public resume() {
+    (this._wsSocket as WS.WebSocket)?.resume();
+  }
+
+  private _createWSSocket() {
+    if (typeof WebSocket !== 'undefined') {
+      this._wsSocket = new WebSocket(this._url);
+    } else {
+      this._wsSocket = new WS(this._url, undefined, this._options);
+    }
+    this._wsSocket.onmessage = this._onWSMessage.bind(this);
+    this._wsSocket.onopen = this._onWSOpen.bind(this);
+    this._wsSocket.onerror = this._onWSError.bind(this);
+    this._wsSocket.onclose = this._onWSClose.bind(this);
+  }
+
+  private _destroyWSSocket() {
+    if (this._wsSocket) {
+      this._wsSocket.onmessage = null;
+      this._wsSocket.onopen = null;
+      this._wsSocket.onerror = null;
+      this._wsSocket.onclose = null;
+      this._wsSocket = undefined;
+    }
+  }
+
+  private _onWSMessage(event: WS.MessageEvent | MessageEvent<any>) {
+    // event.data is guaranteed to be a string here because we only send text frames.
+    // https://developer.mozilla.org/en-US/docs/Web/API/WebSocket/message_event#event_properties
+    this._messageHandler?.(event.data);
+  }
+
+  private _onWSOpen() {
+    // The connection was established successfully. Any future events can now
+    // be forwarded to the client.
+    this._wsConnected = true;
+    this._openHandler?.();
+  }
+
+  private _onWSError(ev: Event) {
+    if (!this._wsConnected) {
+      // The WebSocket connection attempt failed. Switch to Engine.IO.
+      this._destroyWSSocket();
+      this._createEIOSocket();
+      return;
+    }
+
+    // WebSocket error events are deliberately void of information,
+    // see https://websockets.spec.whatwg.org/#eventdef-websocket-error,
+    // so we ignore the incoming event.
+    this._errorHandler?.(new Error("WebSocket error"));
+  }
+
+  private _onWSClose() {
+    this._closeHandler?.();
+  }
+
+  private _createEIOSocket() {
+    this._eioSocket = new EIOSocket(this._url, {
+      path: new URL(this._url).pathname,
+      addTrailingSlash: false,
+      transports: ['polling'],
+      upgrade: false,
+      extraHeaders: this._options?.headers,
+      withCredentials: true,
+    });
+
+    this._eioSocket.on('message', this._onEIOMessage.bind(this));
+    this._eioSocket.on('open', this._onEIOOpen.bind(this));
+    this._eioSocket.on('error', this._onEIOError.bind(this));
+    this._eioSocket.on('close', this._onEIOClose.bind(this));
+  }
+
+  private _onEIOMessage(data: string) {
+    this._messageHandler?.(data);
+  }
+
+  private _onEIOOpen() {
+    this._openHandler?.();
+  }
+
+  private _onEIOError(err: string | Error) {
+    this._errorHandler?.(typeof err === "string" ? new Error(err) : err);
+  }
+
+  private _onEIOClose() {
+    this._closeHandler?.();
+  }
+}

app/client/components/GristWSConnection.ts

@@ -9,6 +9,7 @@ import {addOrgToPath, docUrl, getGristConfig} from 'app/common/urlUtils';
 import {UserAPI} from 'app/common/UserAPI';
 import {Events as BackboneEvents} from 'backbone';
 import {Disposable} from 'grainjs';
+import {GristClientSocket} from './GristClientSocket';
 
 const G = getBrowserGlobals('window');
 const reconnectInterval = [1000, 1000, 2000, 5000, 10000];
@@ -37,7 +38,7 @@ async function getDocWorkerUrl(assignmentId: string|null): Promise<string|null>
 export interface GristWSSettings {
   // A factory function for creating the WebSocket so that we can use from node
   // or browser.
-  makeWebSocket(url: string): WebSocket;
+  makeWebSocket(url: string): GristClientSocket;
 
   // A function for getting the timezone, so the code can be used outside webpack -
   // currently a timezone library is lazy loaded in a way that doesn't quite work
@@ -74,7 +75,7 @@ export interface GristWSSettings {
 export class GristWSSettingsBrowser implements GristWSSettings {
   private _sessionStorage = getSessionStorage();
 
-  public makeWebSocket(url: string) { return new WebSocket(url); }
+  public makeWebSocket(url: string) { return new GristClientSocket(url); }
   public getTimezone()              { return guessTimezone(); }
   public getPageUrl()               { return G.window.location.href; }
   public async getDocWorkerUrl(assignmentId: string|null) {
@@ -125,7 +126,7 @@ export class GristWSConnection extends Disposable {
   private _reconnectTimeout: ReturnType<typeof setTimeout> | null = null;
   private _reconnectAttempts: number = 0;
   private _wantReconnect: boolean = true;
-  private _ws: WebSocket|null = null;
+  private _ws: GristClientSocket|null = null;
 
   // The server sends incremental seqId numbers with each message on the connection, starting with
   // 0. We keep track of them to allow for seamless reconnects.
@@ -211,17 +212,16 @@ export class GristWSConnection extends Disposable {
   }
 
   /**
-   * @event serverMessage Triggered when a message arrives from the server. Callbacks receive
-   *    the raw message data as an additional argument.
+   * Triggered when a message arrives from the server.
    */
-  public onmessage(ev: MessageEvent) {
+  public onmessage(data: string) {
     if (!this._ws) {
       // It's possible to receive a message after we disconnect, at least in tests (where
       // WebSocket is a node library). Ignoring is easier than unsubscribing properly.
       return;
     }
     this._scheduleHeartbeat();
-    this._processReceivedMessage(ev.data, true);
+    this._processReceivedMessage(data, true);
   }
 
   public send(message: any) {
@@ -317,7 +317,7 @@ export class GristWSConnection extends Disposable {
   private _sendHeartbeat() {
     this.send(JSON.stringify({
       beat: 'alive',
-      url: G.window.location.href,
+      url: this._settings.getPageUrl(),
       docId: this._assignmentId,
     }));
   }
@@ -352,8 +352,8 @@ export class GristWSConnection extends Disposable {
 
     this._ws.onmessage = this.onmessage.bind(this);
 
-    this._ws.onerror = (ev: Event|ErrorEvent) => {
-      this._log('GristWSConnection: onerror', 'error' in ev ? String(ev.error) : ev);
+    this._ws.onerror = (err: Error) => {
+      this._log('GristWSConnection: onerror', String(err));
     };
 
     this._ws.onclose = () => {

app/server/lib/Client.ts

@@ -22,7 +22,7 @@ import {fromCallback} from 'app/server/lib/serverUtils';
 import {i18n} from 'i18next';
 import * as crypto from 'crypto';
 import moment from 'moment';
-import * as WebSocket from 'ws';
+import {GristServerSocket} from 'app/server/lib/GristServerSocket';
 
 // How many messages and bytes to accumulate for a disconnected client before booting it.
 // The benefit is that a client who temporarily disconnects and reconnects without missing much,
@@ -97,8 +97,7 @@ export class Client {
   private _missedMessagesTotalLength: number = 0;
   private _destroyTimer: NodeJS.Timer|null = null;
   private _destroyed: boolean = false;
-  private _websocket: WebSocket|null;
-  private _websocketEventHandlers: Array<{event: string, handler: (...args: any[]) => void}> = [];
+  private _websocket: GristServerSocket|null;
   private _org: string|null = null;
   private _profile: UserProfile|null = null;
   private _user: FullUser|undefined = undefined;
@@ -131,18 +130,14 @@ export class Client {
     return this._locale;
   }
 
-  public setConnection(websocket: WebSocket, counter: string|null, browserSettings: BrowserSettings) {
+  public setConnection(websocket: GristServerSocket, counter: string|null, browserSettings: BrowserSettings) {
     this._websocket = websocket;
     this._counter = counter;
     this.browserSettings = browserSettings;
 
-    const addHandler = (event: string, handler: (...args: any[]) => void) => {
-      websocket.on(event, handler);
-      this._websocketEventHandlers.push({event, handler});
-    };
-    addHandler('error', (err: unknown) => this._onError(err));
-    addHandler('close', () => this._onClose());
-    addHandler('message', (msg: string) => this._onMessage(msg));
+    websocket.onerror = (err: Error) => this._onError(err);
+    websocket.onclose = () => this._onClose();
+    websocket.onmessage = (msg: string) => this._onMessage(msg);
   }
 
   /**
@@ -189,7 +184,7 @@ export class Client {
 
   public interruptConnection() {
     if (this._websocket) {
-      this._removeWebsocketListeners();
+      this._websocket.removeAllListeners();
       this._websocket.terminate();  // close() is inadequate when ws routed via loadbalancer
       this._websocket = null;
     }
@@ -359,7 +354,7 @@ export class Client {
       // See also my report at https://stackoverflow.com/a/48411315/328565
       await delay(250);
 
-      if (!this._destroyed && this._websocket?.readyState === WebSocket.OPEN) {
+      if (!this._destroyed && this._websocket?.isOpen) {
         await this._sendToWebsocket(JSON.stringify({...clientConnectMsg, dup: true}));
       }
     } catch (err) {
@@ -604,7 +599,7 @@ export class Client {
   /**
    * Processes an error on the websocket.
    */
-  private _onError(err: unknown) {
+  private _onError(err: Error) {
     this._log.warn(null, "onError", err);
     // TODO Make sure that this is followed by onClose when the connection is lost.
   }
@@ -613,7 +608,7 @@ export class Client {
    * Processes the closing of a websocket.
    */
   private _onClose() {
-    this._removeWebsocketListeners();
+    this._websocket?.removeAllListeners();
 
     // Remove all references to the websocket.
     this._websocket = null;
@@ -629,15 +624,4 @@ export class Client {
       this._destroyTimer = setTimeout(() => this.destroy(), Deps.clientRemovalTimeoutMs);
     }
   }
-
-  private _removeWebsocketListeners() {
-    if (this._websocket) {
-      // Avoiding websocket.removeAllListeners() because WebSocket.Server registers listeners
-      // internally for websockets it keeps track of, and we should not accidentally remove those.
-      for (const {event, handler} of this._websocketEventHandlers) {
-        this._websocket.off(event, handler);
-      }
-      this._websocketEventHandlers = [];
-    }
-  }
 }