Skip to content

Commit

Permalink
Refactor forwarder to remove a limitation in the size of the error code
Browse files Browse the repository at this point in the history
The previous implementation only worked for exit codes of one
character and had repeated magic numbers.
  • Loading branch information
DamienCassou authored and mantoni committed Aug 1, 2024
1 parent fb110f0 commit 12f3afa
Show file tree
Hide file tree
Showing 4 changed files with 85 additions and 29 deletions.
27 changes: 18 additions & 9 deletions lib/forwarder.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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') {
Expand Down
61 changes: 50 additions & 11 deletions lib/forwarder.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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',
Expand All @@ -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',
Expand All @@ -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(
Expand All @@ -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');
});
Expand Down
2 changes: 1 addition & 1 deletion lib/service.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
24 changes: 16 additions & 8 deletions lib/service.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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');
});
});
});

0 comments on commit 12f3afa

Please sign in to comment.