Skip to content

Commit

Permalink
fix uncaughtException error on resolving error for parallel requests …
Browse files Browse the repository at this point in the history
…with the same hostname
  • Loading branch information
oleh-poberezhets committed Oct 19, 2018
1 parent b97cec5 commit bdf291b
Show file tree
Hide file tree
Showing 3 changed files with 89 additions and 16 deletions.
8 changes: 3 additions & 5 deletions src/Lookup.js
Original file line number Diff line number Diff line change
Expand Up @@ -198,14 +198,12 @@ class Lookup {
this._tasksManager.done(key);
});

task.once('error', () => this._tasksManager.done(key));

task.run();
}

task.on('error', error => {
this._tasksManager.done(key);

reject(error);
});
task.on('error', error => reject(error));
task.on('addresses', addresses => resolve(addresses));
});
}
Expand Down
65 changes: 63 additions & 2 deletions tests/Functional/Lookup/_innerResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,5 +100,66 @@ describe('Func: Lookup::_innerResolve', () => {
taskRunSpy.restore();
});
});
})
;

it('must has correct behavior on resolving error for parallel requests with the same hostname', () => {
const ipVersion = 4;

const lookup = new Lookup();

const addressCacheFindSpy = sinon.spy(lookup._addressCache, 'find');
const addressCacheSetSpy = sinon.spy(lookup._addressCache, 'set');

const tasksManagerFindSpy = sinon.spy(lookup._tasksManager, 'find');
const tasksManagerAddSpy = sinon.spy(lookup._tasksManager, 'add');
const tasksManagerDoneSpy = sinon.spy(lookup._tasksManager, 'done');

const taskRunSpy = sinon.spy(ResolveTask.prototype, 'run');

let resolveErrorCount = 0;

const firstRequestResult = lookup._innerResolve(addresses.INVALID_HOST, ipVersion)
.catch(err => {
assert.isPrototypeOf(err, Error);
assert.strictEqual(err.code, 'ENOTFOUND');
resolveErrorCount++;
});

const secondRequestResult = lookup._innerResolve(addresses.INVALID_HOST, ipVersion)
.catch(err => {
assert.isPrototypeOf(err, Error);
assert.strictEqual(err.code, 'ENOTFOUND');
resolveErrorCount++;
});

return Promise.all([firstRequestResult, secondRequestResult])
.then(() => {
assert.strictEqual(resolveErrorCount, 2);

assert.isTrue(addressCacheFindSpy.calledTwice);
assert.isTrue(addressCacheFindSpy.calledWithExactly(`${addresses.INVALID_HOST}_${ipVersion}`));

assert.isTrue(addressCacheSetSpy.notCalled);

assert.isTrue(tasksManagerFindSpy.calledTwice);
assert.isTrue(tasksManagerFindSpy.calledWithExactly(`${addresses.INVALID_HOST}_${ipVersion}`));

assert.isTrue(tasksManagerAddSpy.calledOnce);
assert.strictEqual(tasksManagerAddSpy.getCall(0).args[0], `${addresses.INVALID_HOST}_${ipVersion}`);
assert.instanceOf(tasksManagerAddSpy.getCall(0).args[1], ResolveTask);

assert.isTrue(tasksManagerDoneSpy.calledOnce);
assert.isTrue(tasksManagerDoneSpy.calledWithExactly(`${addresses.INVALID_HOST}_${ipVersion}`));

assert.isTrue(taskRunSpy.calledOnce);

addressCacheFindSpy.restore();
addressCacheSetSpy.restore();

tasksManagerFindSpy.restore();
tasksManagerAddSpy.restore();
tasksManagerDoneSpy.restore();

taskRunSpy.restore();
});
});
});
32 changes: 23 additions & 9 deletions tests/Unit/Lookup/_innerResolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ describe('Unit: Lookup::_innerResolve', () => {
const task = new EventEmitter();
task.run = resolveTaskRunSpy;

const resolveTaskOnSpy = sinon.spy(task, 'on');
const resolveTaskOnSpy = sinon.spy(task, 'on');
const resolveTaskOnceSpy = sinon.spy(task, 'once');

const addressCacheFindSpy = sinon.spy(() => undefined);
const addressCacheSetSpy = sinon.spy();
Expand Down Expand Up @@ -129,23 +130,30 @@ describe('Unit: Lookup::_innerResolve', () => {

assert.isTrue(resolveTaskRunSpy.calledOnce);

assert.strictEqual(resolveTaskOnSpy.callCount, 5);
assert.strictEqual(resolveTaskOnSpy.callCount, 6);
assert.strictEqual(resolveTaskOnceSpy.callCount, 1);

assert.strictEqual(resolveTaskOnSpy.getCall(0).args[0], 'addresses');
assert.instanceOf(resolveTaskOnSpy.getCall(0).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(1).args[0], 'error');
assert.instanceOf(resolveTaskOnSpy.getCall(1).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(2).args[0], 'addresses');
assert.strictEqual(resolveTaskOnSpy.getCall(2).args[0], 'error');
assert.instanceOf(resolveTaskOnSpy.getCall(2).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(3).args[0], 'error');
assert.strictEqual(resolveTaskOnSpy.getCall(3).args[0], 'addresses');
assert.instanceOf(resolveTaskOnSpy.getCall(3).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(4).args[0], 'addresses');
assert.strictEqual(resolveTaskOnSpy.getCall(4).args[0], 'error');
assert.instanceOf(resolveTaskOnSpy.getCall(4).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(5).args[0], 'addresses');
assert.instanceOf(resolveTaskOnSpy.getCall(5).args[1], Function);

assert.strictEqual(resolveTaskOnceSpy.getCall(0).args[0], 'error');
assert.instanceOf(resolveTaskOnceSpy.getCall(0).args[1], Function);

assert.isTrue(tasksManagerFindSpy.calledTwice);
assert.isTrue(tasksManagerAddSpy.calledOnce);
assert.isTrue(tasksManagerDoneSpy.calledOnce);
Expand Down Expand Up @@ -233,16 +241,19 @@ describe('Unit: Lookup::_innerResolve', () => {
assert.isTrue(resolveTaskRunSpy.calledOnce);
assert.isTrue(resolveTaskRunSpy.calledWithExactly());

assert.isTrue(resolveTaskOnSpy.calledThrice);
assert.strictEqual(resolveTaskOnSpy.callCount, 4);

assert.strictEqual(resolveTaskOnSpy.getCall(0).args[0], 'addresses');
assert.instanceOf(resolveTaskOnSpy.getCall(0).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(1).args[0], 'error');
assert.instanceOf(resolveTaskOnSpy.getCall(1).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(2).args[0], 'addresses');
assert.strictEqual(resolveTaskOnSpy.getCall(2).args[0], 'error');
assert.instanceOf(resolveTaskOnSpy.getCall(2).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(3).args[0], 'addresses');
assert.instanceOf(resolveTaskOnSpy.getCall(3).args[1], Function);
});
});

Expand Down Expand Up @@ -316,17 +327,20 @@ describe('Unit: Lookup::_innerResolve', () => {
assert.isTrue(resolveTaskRunSpy.calledOnce);
assert.isTrue(resolveTaskRunSpy.calledWithExactly());

assert.isTrue(resolveTaskOnSpy.calledThrice);
assert.strictEqual(resolveTaskOnSpy.callCount, 4);

assert.strictEqual(resolveTaskOnSpy.getCall(0).args[0], 'addresses');
assert.instanceOf(resolveTaskOnSpy.getCall(0).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(1).args[0], 'error');
assert.instanceOf(resolveTaskOnSpy.getCall(1).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(2).args[0], 'addresses');
assert.strictEqual(resolveTaskOnSpy.getCall(2).args[0], 'error');
assert.instanceOf(resolveTaskOnSpy.getCall(2).args[1], Function);

assert.strictEqual(resolveTaskOnSpy.getCall(3).args[0], 'addresses');
assert.instanceOf(resolveTaskOnSpy.getCall(3).args[1], Function);

done();
});
});
Expand Down

0 comments on commit bdf291b

Please sign in to comment.