Skip to content

Commit

Permalink
🐛 Don't throw errors when an *unused/free* socket has an error in `De…
Browse files Browse the repository at this point in the history
…velry.HttpAgent`
  • Loading branch information
skerit committed Apr 12, 2024
1 parent 8aaad55 commit 1e3a41f
Show file tree
Hide file tree
Showing 3 changed files with 185 additions and 7 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
* Add `Pledge#getResolvedFunction()` method, which can be passed as an old-style callback
* Fix some `Swift` methods not using the correct schedulers
* Implement a custom `done` and `waterfall` static method for the `SwiftPledge` class
* Make `Develry.HttpAgent` classes return sockets from `createConnection`
* Don't throw errors when an *unused/free* socket has an error in `Develry.HttpAgent`

## 0.9.2 (2024-02-25)

Expand Down
31 changes: 26 additions & 5 deletions lib/request_agents.js
Original file line number Diff line number Diff line change
Expand Up @@ -457,7 +457,7 @@ HttpAgent.setMethod(function createSocketId() {
*
* @author Jelle De Loecker <[email protected]>
* @since 0.8.2
* @version 0.8.2
* @version 0.9.3
*/
HttpAgent.setMethod(function initializeSocket(socket, options) {

Expand Down Expand Up @@ -544,6 +544,15 @@ HttpAgent.setMethod(function initializeSocket(socket, options) {

const on_error = (err) => {

const name = this.getName(options);

// If the socket had an error while it was free, ignore the error
if (this.freeSockets[name] && this.freeSockets[name].indexOf(socket) !== -1) {
socket.destroy();
this.removeSocket(socket, options);
return;
}

const listenerCount = socket.listeners('error').length;

this.errorSocketCount++;
Expand Down Expand Up @@ -575,7 +584,12 @@ HttpAgent.setMethod(function initializeSocket(socket, options) {
*
* @author Jelle De Loecker <[email protected]>
* @since 0.8.2
* @version 0.8.2
* @version 0.9.3
*
* @param {Object} options
* @param {Function} on_create
*
* @return {Socket}
*/
HttpAgent.setMethod(function createConnection(options, on_create) {

Expand Down Expand Up @@ -604,6 +618,8 @@ HttpAgent.setMethod(function createConnection(options, on_create) {
if (new_socket) {
on_new_create(null, new_socket);
}

return new_socket;
});

/**
Expand Down Expand Up @@ -664,11 +680,16 @@ const HttpsAgent = Fn.inherits('Develry.HttpAgent', function HttpsAgent(options)
*
* @author Jelle De Loecker <[email protected]>
* @since 0.8.2
* @version 0.8.2
* @version 0.9.3
*
* @param {Object} options
* @param {Function} on_create
*
* @return {Socket}
*/
HttpsAgent.setMethod(function createConnection(options) {
HttpsAgent.setMethod(function createConnection(options, on_create) {

const socket = CREATE_HTTPS_CONNECTION.call(this, options);
const socket = CREATE_HTTPS_CONNECTION.call(this, options, on_create);

this.initializeSocket(socket, options);
return socket;
Expand Down
159 changes: 157 additions & 2 deletions test/request_agents.js
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ describe('HttpAgent', function() {
recreateAgentKeepAlive();

app = http.createServer((req, res) => {

const endResponse = (message) => {
res.end(message);
};

if (req.url === '/error') {
res.destroy();
return;
Expand All @@ -81,12 +86,12 @@ describe('HttpAgent', function() {

if (info.query.timeout) {
setTimeout(() => {
res.end(info.query.timeout);
endResponse(info.query.timeout);
}, parseInt(info.query.timeout));
return;
}

res.end(JSON.stringify({
endResponse(JSON.stringify({
info,
url: req.url,
headers: req.headers,
Expand Down Expand Up @@ -2152,6 +2157,156 @@ describe('HttpsAgent', function() {
});
});

it('should support a lot of requests', async function () {

this.timeout(30000);

let agentkeepalive = new HttpsAgent({
freeSocketTimeout: 100,
timeout: 20,
maxSockets: 30,
});

let count = 0;

const capem = fs.readFileSync(__dirname + '/assets/ca.pem');

function makeRequest(i) {

let pledge = new Blast.Classes.Pledge();

const req = https.get({
agent: agentkeepalive,
headers: {
'rcount' : '' + i,
},
port,
path: '/?timeout=5',
ca: capem,
rejectUnauthorized: false,
}, res => {

let data = '';

res.on('data', chunk => {
data += chunk;
});

res.on('end', () => {
count++;
pledge.resolve(data);
});
});

req.on('error', err => {
pledge.reject(err);
});

return pledge;
}

let promises = [];

for (let i = 0; i < 300; i++) {
promises.push(makeRequest(i));

if (i % 5 == 0) {
await Pledge.after(6);
}
}

await Pledge.all(promises);

assert.strictEqual(count, 300);
});

it('should perform requests in series if there is only 1 socket to reuse', async function () {

this.timeout(30000);

let agentkeepalive = new HttpsAgent({
freeSocketTimeout: 100,
timeout: 100,
maxSockets: 1,
});

let count = 0;

const capem = fs.readFileSync(__dirname + '/assets/ca.pem');

let finished_indexes = [];

function makeRequest(i, timeout) {

let pledge = new Blast.Classes.Pledge();

if (!timeout) {
timeout = Number.random(1, 10);
}

const req = https.get({
agent: agentkeepalive,
headers: {
'rcount' : '' + i,
},
port,
path: '/?timeout=' + timeout,
ca: capem,
rejectUnauthorized: false,
}, res => {

let data = '';

res.on('data', chunk => {
data += chunk;
});

res.on('end', () => {
finished_indexes.push(i);
count++;
pledge.resolve(data);
});
});

req.on('error', err => {
pledge.reject(err);
});

return pledge;
}

let promises = [];

for (let i = 0; i < 30; i++) {
promises.push(makeRequest(i));
}

await Pledge.all(promises);

assert.deepStrictEqual(finished_indexes, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29]);

// Create an agent with more sockets
agentkeepalive = new HttpsAgent({
freeSocketTimeout: 100,
timeout: 100,
maxSockets: 10,
});

promises = [];
finished_indexes = [];

// Create requests with a fixed timeout
for (let i = 0; i < 30; i++) {
promises.push(makeRequest(i, 2));
}

await Pledge.all(promises);

// This time, the requests should be done in parallel,
// and thus the indexes should be different
assert.notDeepStrictEqual(finished_indexes, [0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29]);
});

it('should GET / success with 200 status', done => {
https.get({
agent: agentkeepalive,
Expand Down

0 comments on commit 1e3a41f

Please sign in to comment.