Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor forwarder to remove a limitation in the size of the error code #306

Merged
merged 1 commit into from
Aug 1, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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');
});
});
});
Loading