Repository: mantoni/eslint_d.js
Pull request #306 (2024-08-01 19:00:35)

Refactor forwarder to remove a limitation in the size of the error code
The previous implementation only worked for exit codes of one character and had repeated magic numbers.

This is mainly the work of @DamienCassou from #301. I changed the exit code token to have fixed length of 7 characters, padding the exit code with zeros. This allowed to simplify the response handling logic a bit.


Patch:
diff --git a/lib/forwarder.js b/lib/forwarder.js
index 1f60912..c64ae5c 100644
--- a/lib/forwarder.js
+++ b/lib/forwarder.js
@@ -7,6 +7,9 @@ import { removeConfig } from '../lib/config.js';
  * @import { Resolver} from '../lib/resolver.js'
  */
 
+const EXIT_TOKEN_REGEXP = new RegExp(/EXIT([0-9]{3})/);
+const EXIT_TOKEN_LENGTH = 7;
+
 /**
  * @param {Resolver} resolver
  * @param {Config} config
@@ -36,20 +39,26 @@ export async function forwardToDaemon(resolver, config) {
       let chunk = '';
       while ((chunk = socket.read()) !== null) {
         content += chunk;
-        if (content.length > 5) {
-          process.stdout.write(content.substring(0, content.length - 5));
-          content = content.substring(content.length - 5);
+        if (content.length > EXIT_TOKEN_LENGTH) {
+          const message_length = content.length - EXIT_TOKEN_LENGTH;
+          // Write everything we are sure doesn't contain the termination code:
+          process.stdout.write(content.substring(0, message_length));
+          // Keep only what we haven't written yet:
+          content = content.substring(message_length);
         }
       }
     })
     .on('end', () => {
-      if (content.startsWith('EXIT')) {
-        process.exitCode = Number(content.slice(4));
-      } else {
-        process.stdout.write(content);
-        console.error('eslint_d: unexpected response');
-        process.exitCode = 1;
+      // The remaining 'content' must be the termination code:
+      const match = content.match(EXIT_TOKEN_REGEXP);
+      if (match) {
+        process.exitCode = Number(match[1]);
+        return;
       }
+
+      process.stdout.write(content);
+      console.error('eslint_d: unexpected response');
+      process.exitCode = 1;
     })
     .on('error', async (err) => {
       if (err['code'] === 'ECONNREFUSED') {
diff --git a/lib/service.js b/lib/service.js
index c90f3da..5df0e50 100644
--- a/lib/service.js
+++ b/lib/service.js
@@ -60,7 +60,7 @@ export function createService(resolver, token) {
           process.stdout.write = stdout_write;
           process.stderr.write = stderr_write;
           /* eslint-enable require-atomic-updates */
-          con.end(`EXIT${code}`);
+          con.end(`EXIT${String(code).padStart(3, '0')}`);
         }
       })
       .on('error', () => {


Test patch:
diff --git a/lib/forwarder.test.js b/lib/forwarder.test.js
index 0a56eff..cc4c5f8 100644
--- a/lib/forwarder.test.js
+++ b/lib/forwarder.test.js
@@ -85,7 +85,7 @@ describe('lib/forwarder', () => {
       assert.calledWith(socket.write, text);
     });
 
-    it('forwards socket response to stdout, except for the last 5 characters', () => {
+    it('forwards socket response to stdout', () => {
       const chunks = ['response ', 'from daemon'];
       sinon.replace(
         socket,
@@ -96,14 +96,16 @@ describe('lib/forwarder', () => {
 
       forwardToDaemon(resolver, config);
       socket.on.firstCall.callback(); // readable
+      socket.on.secondCall.callback(); // end
 
-      assert.calledTwice(process.stdout.write);
-      assert.calledWith(process.stdout.write, 'resp');
-      assert.calledWith(process.stdout.write, 'onse from d');
+      assert.calledThrice(process.stdout.write);
+      assert.calledWith(process.stdout.write, 're');
+      assert.calledWith(process.stdout.write, 'sponse from');
+      assert.calledWith(process.stdout.write, ' daemon');
     });
 
-    it('handles EXIT0 from response', () => {
-      const chunks = ['response from daemonEXIT0'];
+    it('handles "EXIT000" from response', () => {
+      const chunks = ['response from daemonEXIT000'];
       sinon.replace(
         socket,
         'read',
@@ -115,13 +117,13 @@ describe('lib/forwarder', () => {
       socket.on.firstCall.callback(); // readable
       socket.on.secondCall.callback(); // end
 
-      assert.calledOnceWith(process.stdout.write, 'response from daemon');
+      assert.calledWith(process.stdout.write, 'response from daemon');
       assert.equals(process.exitCode, 0);
       refute.called(console.error);
     });
 
-    it('handles EXIT1 from response', () => {
-      const chunks = ['response from daemonEXIT1'];
+    it('handles "EXIT001" from response', () => {
+      const chunks = ['response from daemonEXIT001'];
       sinon.replace(
         socket,
         'read',
@@ -138,6 +140,43 @@ describe('lib/forwarder', () => {
       refute.called(console.error);
     });
 
+    it('handles "EXIT123" from response', () => {
+      const chunks = ['response from daemonEXIT123'];
+      sinon.replace(
+        socket,
+        'read',
+        sinon.fake(() => (chunks.length ? chunks.shift() : null))
+      );
+      sinon.replace(process.stdout, 'write', sinon.fake());
+
+      forwardToDaemon(resolver, config);
+      socket.on.firstCall.callback(); // readable
+      socket.on.secondCall.callback(); // end
+
+      assert.calledWith(process.stdout.write, 'response from daemon');
+      assert.equals(process.exitCode, 123);
+      refute.called(console.error);
+    });
+
+    it('handles "EXIT001" inside response', () => {
+      const chunks = ['response EXIT001', ' from daemonEXIT002'];
+      sinon.replace(
+        socket,
+        'read',
+        sinon.fake(() => (chunks.length ? chunks.shift() : null))
+      );
+      sinon.replace(process.stdout, 'write', sinon.fake());
+
+      forwardToDaemon(resolver, config);
+      socket.on.firstCall.callback(); // readable
+      socket.on.secondCall.callback(); // end
+
+      assert.calledWith(process.stdout.write, 'response ');
+      assert.calledWith(process.stdout.write, 'EXIT001 from daemon');
+      assert.equals(process.exitCode, 2);
+      refute.called(console.error);
+    });
+
     it('logs error and sets exitCode to 1 if response does not end with EXIT marker', () => {
       const chunks = ['response from daemon'];
       sinon.replace(
@@ -151,8 +190,8 @@ describe('lib/forwarder', () => {
       socket.on.firstCall.callback(); // readable
       socket.on.secondCall.callback(); // end
 
-      assert.calledWith(process.stdout.write, 'response from d');
-      assert.calledWith(process.stdout.write, 'aemon');
+      assert.calledWith(process.stdout.write, 'response from');
+      assert.calledWith(process.stdout.write, ' daemon');
       assert.equals(process.exitCode, 1);
       assert.calledOnceWith(console.error, 'eslint_d: unexpected response');
     });
diff --git a/lib/service.test.js b/lib/service.test.js
index 0dbb518..62daf9c 100644
--- a/lib/service.test.js
+++ b/lib/service.test.js
@@ -111,36 +111,44 @@ describe('lib/service', () => {
       assert.calledOnceWith(eslint.execute, [], 'some text', true);
     });
 
-    it('ends connection with "EXIT0" if eslint returns 0', async () => {
+    it('ends connection with "EXIT000" if eslint returns 0', async () => {
       send(token, '3', '/', []);
 
       await eslint_promise.resolve(0);
       refute.called(con.write);
-      assert.calledOnceWith(con.end, 'EXIT0');
+      assert.calledOnceWith(con.end, 'EXIT000');
     });
 
-    it('ends connection with "EXIT1" if eslint returns 1', async () => {
+    it('ends connection with "EXIT001" if eslint returns 1', async () => {
       send(token, '3', '/', []);
 
       await eslint_promise.resolve(1);
       refute.called(con.write);
-      assert.calledOnceWith(con.end, 'EXIT1');
+      assert.calledOnceWith(con.end, 'EXIT001');
     });
 
-    it('ends connection with "EXIT2" if eslint returns 2', async () => {
+    it('ends connection with "EXIT002" if eslint returns 2', async () => {
       send(token, '3', '/', []);
 
       await eslint_promise.resolve(2);
       refute.called(con.write);
-      assert.calledOnceWith(con.end, 'EXIT2');
+      assert.calledOnceWith(con.end, 'EXIT002');
     });
 
-    it('ends connection with "EXIT1" if eslint throws', async () => {
+    it('ends connection with "EXIT123" if eslint returns 123', async () => {
+      send(token, '3', '/', []);
+
+      await eslint_promise.resolve(123);
+      refute.called(con.write);
+      assert.calledOnceWith(con.end, 'EXIT123');
+    });
+
+    it('ends connection with "EXIT001" if eslint throws', async () => {
       send(token, '3', '/', []);
 
       await eslint_promise.reject(new Error('Ouch!'));
       assert.calledOnceWith(con.write, 'Error: Ouch!');
-      assert.calledOnceWith(con.end, 'EXIT1');
+      assert.calledOnceWith(con.end, 'EXIT001');
     });
   });
 });