updated ws dependency and slightly improved client side error handlin…

…g, hung uploads will error instead of hang forever

app/api.js

@@ -147,7 +147,16 @@ function asyncInitWebSocket(server) {
 
 function listenForResponse(ws, canceller) {
   return new Promise((resolve, reject) => {
+    function handleClose(event) {
+      // a 'close' event before a 'message' event means the request failed
+      ws.removeEventListener('message', handleMessage);
+      const error = canceller.cancelled
+        ? canceller.error
+        : new Error('connection closed');
+      reject(error);
+    }
     function handleMessage(msg) {
+      ws.removeEventListener('close', handleClose);
       try {
         const response = JSON.parse(msg.data);
         if (response.error) {
@@ -156,13 +165,11 @@ function listenForResponse(ws, canceller) {
           resolve(response);
         }
       } catch (e) {
-        ws.close();
-        canceller.cancelled = true;
-        canceller.error = e;
         reject(e);
       }
     }
     ws.addEventListener('message', handleMessage, { once: true });
+    ws.addEventListener('close', handleClose, { once: true });
   });
 }
 
@@ -215,7 +222,10 @@ async function upload(
       onprogress(size);
       size += buf.length;
       state = await reader.read();
-      while (ws.bufferedAmount > ECE_RECORD_SIZE * 2) {
+      while (
+        ws.bufferedAmount > ECE_RECORD_SIZE * 2 &&
+        ws.readyState === WebSocket.OPEN
+      ) {
         await delay();
       }
     }

package.json

@@ -137,6 +137,7 @@
     "webpack-unassert-loader": "^1.2.0"
   },
   "dependencies": {
+    "@dannycoates/express-ws": "^5.0.1",
     "@fluent/bundle": "^0.13.0",
     "@fluent/langneg": "^0.3.0",
     "@google-cloud/storage": "^3.0.4",
@@ -146,7 +147,6 @@
     "cldr-core": "^35.1.0",
     "convict": "^5.1.0",
     "express": "^4.17.1",
-    "express-ws": "github:dannycoates/express-ws",
     "fxa-geodb": "^1.0.4",
     "helmet": "^3.20.0",
     "mkdirp": "^0.5.1",
@@ -155,8 +155,7 @@
     "raven": "^2.6.4",
     "redis": "^2.8.0",
     "selenium-standalone": "^6.15.6",
-    "ua-parser-js": "^0.7.20",
-    "websocket-stream": "^5.5.0"
+    "ua-parser-js": "^0.7.20"
   },
   "availableLanguages": [
     "en-US",

server/bin/dev.js

@@ -3,7 +3,7 @@ const routes = require('../routes');
 const pages = require('../routes/pages');
 const tests = require('../../test/frontend/routes');
 const express = require('express');
-const expressWs = require('express-ws');
+const expressWs = require('@dannycoates/express-ws');
 const morgan = require('morgan');
 const config = require('../config');
 

server/bin/prod.js

@@ -4,7 +4,7 @@ const Raven = require('raven');
 const config = require('../config');
 const routes = require('../routes');
 const pages = require('../routes/pages');
-const expressWs = require('express-ws');
+const expressWs = require('@dannycoates/express-ws');
 
 if (config.sentry_dsn) {
   Raven.config(config.sentry_dsn).install();

server/bin/test.js

@@ -2,7 +2,7 @@ const assets = require('../../common/assets');
 const routes = require('../routes');
 const pages = require('../routes/pages');
 const tests = require('../../test/frontend/routes');
-const expressWs = require('express-ws');
+const expressWs = require('@dannycoates/express-ws');
 
 module.exports = function(app, devServer) {
   assets.setMiddleware(devServer.middleware);

server/routes/ws.js

@@ -3,12 +3,11 @@ const storage = require('../storage');
 const config = require('../config');
 const mozlog = require('../log');
 const Limiter = require('../limiter');
-const wsStream = require('websocket-stream/stream');
 const fxa = require('../fxa');
 const { statUploadEvent } = require('../amplitude');
 const { encryptedSize } = require('../../app/utils');
 
-const { Duplex } = require('stream');
+const { Transform } = require('stream');
 
 const log = mozlog('send.upload');
 
@@ -76,25 +75,19 @@ module.exports = function(ws, req) {
         })
       );
       const limiter = new Limiter(encryptedSize(maxFileSize));
-      const flowControl = new Duplex({
-        read() {
-          ws.resume();
-        },
-        write(chunk, encoding, callback) {
+      const eof = new Transform({
+        transform: function(chunk, encoding, callback) {
           if (chunk.length === 1 && chunk[0] === 0) {
             this.push(null);
           } else {
-            if (!this.push(chunk)) {
-              ws.pause();
-            }
+            this.push(chunk);
           }
           callback();
         }
       });
+      const wsStream = ws.constructor.createWebSocketStream(ws);
 
-      fileStream = wsStream(ws, { binary: true })
-        .pipe(flowControl)
-        .pipe(limiter); // limiter needs to be the last in the chain
+      fileStream = wsStream.pipe(eof).pipe(limiter); // limiter needs to be the last in the chain
 
       await storage.set(newId, fileStream, meta, timeLimit);
 
@@ -126,8 +119,8 @@ module.exports = function(ws, req) {
             error: e === 'limit' ? 413 : 500
           })
         );
-        ws.close();
       }
     }
+    ws.close();
   });
 };

test/testServer.js

@@ -6,7 +6,7 @@ module.exports = {
       const webpack = require('webpack');
       const middleware = require('webpack-dev-middleware');
       const express = require('express');
-      const expressWs = require('express-ws');
+      const expressWs = require('@dannycoates/express-ws');
       const assets = require('../common/assets');
       const routes = require('../server/routes');
       const tests = require('./frontend/routes');