Skip to content

Commit

Permalink
test: fix http-agent-destroyed-socket cb not firing
Browse files Browse the repository at this point in the history
A whole part of the test-http-agent-destroyed-socket test
was not running as the semantics of http events changed
slightly and were no longer triggering the expected event.
Instead listen to the same event on the socket to verify
that the code being tested is still working as expected.

PR-URL: #20006
Fixes: #8613
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
  • Loading branch information
apapirovski authored and jasnell committed Apr 17, 2018
1 parent ed45a8b commit 77f3c1f
Showing 1 changed file with 2 additions and 15 deletions.
17 changes: 2 additions & 15 deletions test/parallel/test-http-agent-destroyed-socket.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,24 +44,11 @@ const server = http.createServer(common.mustCall((req, res) => {
// assert request2 is queued in the agent
const key = agent.getName(requestOptions);
assert.strictEqual(agent.requests[key].length, 1);
request1.socket.on('close', common.mustCall());
response.resume();
response.on('end', common.mustCall(() => {
//
// THE IMPORTANT PART
//
// It is possible for the socket to get destroyed and other work
// to run before the 'close' event fires because it happens on
// nextTick. This example is contrived because it destroys the
// socket manually at just the right time, but at Voxer we have
// seen cases where the socket is destroyed by non-user code
// then handed out again by an agent *before* the 'close' event
// is triggered.
request1.socket.destroy();

// TODO(jasnell): This close event does not appear to be triggered.
// is it necessary?
response.once('close', () => {
response.socket.once('close', common.mustCall(() => {
// assert request2 was removed from the queue
assert(!agent.requests[key]);
process.nextTick(() => {
Expand All @@ -70,7 +57,7 @@ const server = http.createServer(common.mustCall((req, res) => {
assert.notStrictEqual(request1.socket, request2.socket);
assert(!request2.socket.destroyed, 'the socket is destroyed');
});
});
}));
}));
}));

Expand Down

0 comments on commit 77f3c1f

Please sign in to comment.