Refactor server.js

mantoni · Aug 21, 2018 · 685fcf0 · 685fcf0
1 parent 88cb781
commit 685fcf0
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 28 deletions.
diff --git a/lib/server.js b/lib/server.js
@@ -5,18 +5,44 @@ const net = require('net');
 const portfile = require('./portfile');
 const linter = require('./linter');
 
-function forceClose(con) {
+function fail(con, message) {
   try {
-    con.end('Server is stopping...\n# exit 1');
+    con.end(`${message}\n# exit 1`);
   } catch (ignore) {
     // Nothing we can do
   }
 }
 
+function remove(connections, con) {
+  const p = connections.indexOf(con);
+  if (p !== -1) {
+    connections.splice(p, 1);
+  }
+}
+
+function parseData(data) {
+  if (data.substring(0, 1) === '{') {
+    return JSON.parse(data);
+  }
+
+  const newlineIndex = data.indexOf('\n');
+  let text;
+  if (newlineIndex >= 0) {
+    text = data.slice(newlineIndex + 1);
+    data = data.slice(0, newlineIndex);
+  }
+  const parts = data.split(' ');
+  return {
+    cwd: parts[0],
+    args: parts.slice(1),
+    text
+  };
+}
+
 exports.start = function () {
 
   const token = crypto.randomBytes(8).toString('hex');
-  let openConnections = [];
+  const open_connections = [];
 
   const server = net.createServer({
     allowHalfOpen: true
@@ -26,9 +52,7 @@ exports.start = function () {
       data += chunk;
     });
     con.on('end', () => {
-      openConnections = openConnections.filter((c) => {
-        return c !== con;
-      });
+      remove(open_connections, con);
       if (!data) {
         con.end();
         return;
@@ -40,7 +64,7 @@ exports.start = function () {
       }
       data = data.substring(p + 1);
       if (data === 'stop') {
-        openConnections.forEach(forceClose);
+        open_connections.forEach((con) => fail(con, 'Server is stopping...'));
         con.end();
         server.close();
         return;
@@ -49,33 +73,21 @@ exports.start = function () {
         con.end(linter.getStatus());
         return;
       }
-      let cwd, args, text;
-      if (data.substring(0, 1) === '{') {
-        const json = JSON.parse(data);
-        cwd = json.cwd;
-        args = json.args;
-        text = json.text;
-      } else {
-        const newlineIndex = data.indexOf('\n');
-        if (newlineIndex >= 0) {
-          text = data.slice(newlineIndex + 1);
-          data = data.slice(0, newlineIndex);
-        }
-        const parts = data.split(' ');
-        cwd = parts[0];
-        args = parts.slice(1);
-      }
+      const { cwd, args, text } = parseData(data);
+      let result;
       try {
-        con.write(linter.lint(cwd, args, text));
+        result = linter.lint(cwd, args, text);
       } catch (e) {
-        con.write(`${e.toString()}\n# exit 1`);
+        fail(con, e.toString());
+        return;
       }
+      con.write(result);
       con.end();
     });
   });
 
   server.on('connection', (con) => {
-    openConnections = openConnections.concat([con]);
+    open_connections.push(con);
   });
 
   server.listen(0, '127.0.0.1', () => {

diff --git a/test/server-test.js b/test/server-test.js
@@ -202,8 +202,17 @@ describe('server', () => {
 
       request(connection, `${token} ${JSON.stringify(json)}`);
 
-      assert.calledOnceWith(connection.write, 'Error: Whatever\n# exit 1');
-      assert.calledOnce(connection.end);
+      assert.calledOnceWith(connection.end, 'Error: Whatever\n# exit 1');
+    });
+
+    it('does not throw if connection died after exception from linter', () => {
+      sinon.replace(linter, 'lint', sinon.fake.throws(new Error('Whatever')));
+      connection.end = sinon.fake.throws(new Error('Oh dear!'));
+      start();
+
+      refute.exception(() => {
+        request(connection, `${token} ${JSON.stringify(json)}`);
+      });
     });
 
   });