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

net,tls: add abort signal support to connect #37735

Closed
Closed
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
net,tls: add abort signal support to connect
Add documentation for net.connect AbortSignal,
and add the support to tls.connect as well
  • Loading branch information
Linkgoron committed Mar 20, 2021
commit ce73ba194bb0ae66abef986b765a5301acf372fa
6 changes: 6 additions & 0 deletions doc/api/net.md
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,10 @@ it to interact with the client.
### `new net.Socket([options])`
<!-- YAML
added: v0.3.4
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/37735
description: AbortSignal support was added.
-->

* `options` {Object} Available options are:
Expand All @@ -508,6 +512,8 @@ added: v0.3.4
otherwise ignored. **Default:** `false`.
* `writable` {boolean} Allow writes on the socket when an `fd` is passed,
otherwise ignored. **Default:** `false`.
* `signal` {AbortSignal} An Abort signal that may be used to destroy the
socket.
* Returns: {net.Socket}

Creates a new socket object.
Expand Down
2 changes: 2 additions & 0 deletions lib/_tls_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -515,6 +515,7 @@ function TLSSocket(socket, opts) {
manualStart: true,
highWaterMark: tlsOptions.highWaterMark,
onread: !socket ? tlsOptions.onread : null,
signal: tlsOptions.signal,
}]);

// Proxy for API compatibility
Expand Down Expand Up @@ -1627,6 +1628,7 @@ exports.connect = function connect(...args) {
pskCallback: options.pskCallback,
highWaterMark: options.highWaterMark,
onread: options.onread,
signal: options.signal,
});

tlssock[kConnectOptions] = options;
Expand Down
45 changes: 45 additions & 0 deletions test/parallel/test-http2-client-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -240,3 +240,48 @@ const { getEventListeners } = require('events');
req.on('close', common.mustCall(() => server.close()));
}));
}


// Destroy ClientHttpSession with AbortSignal
{
function testH2ConnectAbort(secure) {
const server = secure ? h2.createSecureServer() : h2.createServer();
const controller = new AbortController();

server.on('stream', common.mustNotCall());
server.listen(0, common.mustCall(() => {
const { signal } = controller;
const protocol = secure ? 'https' : 'http';
const client = h2.connect(`${protocol}://localhost:${server.address().port}`, {
signal,
});
client.on('close', common.mustCall());
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);

client.on('error', common.mustCall(common.mustCall((err) => {
assert.strictEqual(err.code, 'ABORT_ERR');
assert.strictEqual(err.name, 'AbortError');
})));

const req = client.request({}, {});
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);

req.on('error', common.mustCall((err) => {
assert.strictEqual(err.code, 'ERR_HTTP2_STREAM_CANCEL');
assert.strictEqual(err.name, 'Error');
assert.strictEqual(req.aborted, false);
assert.strictEqual(req.destroyed, true);
}));
req.on('close', common.mustCall(() => server.close()));

assert.strictEqual(req.aborted, false);
assert.strictEqual(req.destroyed, false);
// Signal listener attached
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);

controller.abort();
}));
}
testH2ConnectAbort(false);
testH2ConnectAbort(true);
}
55 changes: 55 additions & 0 deletions test/parallel/test-https-abortcontroller.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const options = {
cert: fixtures.readKey('agent1-cert.pem')
};

// Check async post-aborted
(async () => {
const { port, server } = await new Promise((resolve) => {
const server = https.createServer(options, () => {});
Expand All @@ -38,3 +39,57 @@ const options = {
server.close();
}
})().then(common.mustCall());

// Check sync post-aborted signal
(async () => {
const { port, server } = await new Promise((resolve) => {
const server = https.createServer(options, () => {});
server.listen(0, () => resolve({ port: server.address().port, server }));
});
try {
const ac = new AbortController();
const { signal } = ac;
const req = https.request({
host: 'localhost',
port,
path: '/',
method: 'GET',
rejectUnauthorized: false,
signal,
});
assert.strictEqual(getEventListeners(ac.signal, 'abort').length, 1);
ac.abort();
const [ err ] = await once(req, 'error');
assert.strictEqual(err.name, 'AbortError');
assert.strictEqual(err.code, 'ABORT_ERR');
} finally {
server.close();
}
})().then(common.mustCall());

// Check pre-aborted signal
(async () => {
const { port, server } = await new Promise((resolve) => {
const server = https.createServer(options, () => {});
server.listen(0, () => resolve({ port: server.address().port, server }));
});
try {
const ac = new AbortController();
const { signal } = ac;
ac.abort();
const req = https.request({
host: 'localhost',
port,
path: '/',
method: 'GET',
rejectUnauthorized: false,
signal,
});
assert.strictEqual(getEventListeners(ac.signal, 'abort').length, 0);
const [ err ] = await once(req, 'error');
assert.strictEqual(err.name, 'AbortError');
assert.strictEqual(err.code, 'ABORT_ERR');
} finally {
server.close();
}
})().then(common.mustCall());
87 changes: 87 additions & 0 deletions test/parallel/test-https-agent-abort-controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
'use strict';
const common = require('../common');
if (!common.hasCrypto)
common.skip('missing crypto');

const assert = require('assert');
const https = require('https');
const { once } = require('events');
const Agent = https.Agent;
const fixtures = require('../common/fixtures');

const { getEventListeners } = require('events');
const agent = new Agent();

const options = {
key: fixtures.readKey('agent1-key.pem'),
cert: fixtures.readKey('agent1-cert.pem')
};

const server = https.createServer(options);

server.listen(0, common.mustCall(async () => {
const port = server.address().port;
const host = 'localhost';
const options = {
port: port,
host: host,
rejectUnauthorized: false,
_agentKey: agent.getName({ port, host })
};

async function postCreateConnection() {
const ac = new AbortController();
const { signal } = ac;
const connection = agent.createConnection({ ...options, signal });
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);
ac.abort();
const [err] = await once(connection, 'error');
assert.strictEqual(err.name, 'AbortError');
}

async function preCreateConnection() {
const ac = new AbortController();
const { signal } = ac;
ac.abort();
const connection = agent.createConnection({ ...options, signal });
const [err] = await once(connection, 'error');
assert.strictEqual(err.name, 'AbortError');
}


async function agentAsParam() {
const ac = new AbortController();
const { signal } = ac;
const request = https.get({
port: server.address().port,
path: '/hello',
agent: agent,
signal,
});
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);
ac.abort();
const [err] = await once(request, 'error');
assert.strictEqual(err.name, 'AbortError');
}

async function agentAsParamPreAbort() {
const ac = new AbortController();
const { signal } = ac;
ac.abort();
const request = https.get({
port: server.address().port,
path: '/hello',
agent: agent,
signal,
});
assert.strictEqual(getEventListeners(signal, 'abort').length, 0);
const [err] = await once(request, 'error');
assert.strictEqual(err.name, 'AbortError');
}

await postCreateConnection();
await preCreateConnection();
await agentAsParam();
await agentAsParamPreAbort();
server.close();
}));
96 changes: 96 additions & 0 deletions test/parallel/test-net-connect-abort-controller.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,96 @@
'use strict';
const common = require('../common');
const net = require('net');
const assert = require('assert');
const server = net.createServer();
const { getEventListeners, once } = require('events');

const liveConnections = new Set();

server.listen(0, common.mustCall(async () => {
const port = server.address().port;
const host = 'localhost';
const socketOptions = (signal) => ({ port, host, signal });
server.on('connection', (connection) => {
liveConnections.add(connection);
connection.on('close', () => {
liveConnections.delete(connection);
});
});

const assertAbort = async (socket, testName) => {
try {
await once(socket, 'close');
assert.fail(`close ${testName} should have thrown`);
} catch (err) {
assert.strictEqual(err.name, 'AbortError');
}
};

async function postAbort() {
const ac = new AbortController();
const { signal } = ac;
const socket = net.connect(socketOptions(signal));
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);
ac.abort();
await assertAbort(socket, 'postAbort');
}

async function preAbort() {
const ac = new AbortController();
const { signal } = ac;
ac.abort();
const socket = net.connect(socketOptions(signal));
assert.strictEqual(getEventListeners(signal, 'abort').length, 0);
await assertAbort(socket, 'preAbort');
}

async function tickAbort() {
const ac = new AbortController();
const { signal } = ac;
setImmediate(() => ac.abort());
const socket = net.connect(socketOptions(signal));
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);
await assertAbort(socket, 'tickAbort');
}

async function testConstructor() {
const ac = new AbortController();
const { signal } = ac;
ac.abort();
const socket = new net.Socket(socketOptions(signal));
assert.strictEqual(getEventListeners(signal, 'abort').length, 0);
await assertAbort(socket, 'testConstructor');
}

async function testConstructorPost() {
const ac = new AbortController();
const { signal } = ac;
const socket = new net.Socket(socketOptions(signal));
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);
ac.abort();
await assertAbort(socket, 'testConstructorPost');
}

async function testConstructorPostTick() {
const ac = new AbortController();
const { signal } = ac;
const socket = new net.Socket(socketOptions(signal));
assert.strictEqual(getEventListeners(signal, 'abort').length, 1);
setImmediate(() => ac.abort());
await assertAbort(socket, 'testConstructorPostTick');
}

await postAbort();
await preAbort();
await tickAbort();
await testConstructor();
await testConstructorPost();
await testConstructorPostTick();

// Killing the net.socket without connecting hangs the server.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably easier to call socket.setTimeout with a low timeout - but probably what you did is also fine

for (const connection of liveConnections) {
connection.destroy();
}
server.close(common.mustCall());
}));
Loading