From f24b69142aa433c0fbef7529d3665793773040e0 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:26:17 +0530 Subject: [PATCH] fix: handle request errors when fetching CODEOWNERS getCodeOwnersFile() listened for an 'err' event on the https request, which ClientRequest never emits; failures are emitted as 'error'. Any network-level failure (DNS, ECONNRESET, socket hang up, TLS) therefore had no listener, was thrown as an uncaught exception and terminated the bot through the uncaughtException handler in server.js, instead of rejecting the promise and letting the surrounding Aigle.retry() retry. Listen for 'error' on the request, also reject on errors emitted by the response stream (e.g. a connection dropped mid-body), and drain the body of non-200 responses so the socket is released. Add a unit test that makes the request fail at the network level and expects getCodeOwnersFile() to reject; it fails with an uncaught exception without this change. Fixes: https://github.com/nodejs/github-bot/issues/705 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- lib/node-repo.js | 8 +++++++- test/unit/node-repo-owners.test.js | 22 ++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/node-repo.js b/lib/node-repo.js index c21210e6..12982f2a 100644 --- a/lib/node-repo.js +++ b/lib/node-repo.js @@ -117,6 +117,8 @@ function getCodeOwnersFile (url, { logger }) { return new Promise((resolve, reject) => { https.get(url, (res) => { if (res.statusCode !== 200) { + // discard the response body so the socket is released + res.resume() logger.error(`Status code: ${res.statusCode}`, 'Error retrieving OWNERS') return reject(res.statusCode) } @@ -127,7 +129,11 @@ function getCodeOwnersFile (url, { logger }) { res.on('end', () => { return resolve(body) }) - }).on('err', (err) => { + res.on('error', (err) => { + logger.error(err, 'Error retrieving OWNERS') + return reject(err) + }) + }).on('error', (err) => { logger.error(err, 'Error retrieving OWNERS') return reject(err) }) diff --git a/test/unit/node-repo-owners.test.js b/test/unit/node-repo-owners.test.js index c62f2c08..483384e4 100644 --- a/test/unit/node-repo-owners.test.js +++ b/test/unit/node-repo-owners.test.js @@ -169,6 +169,28 @@ test('getCodeOwnersFile fail', async (t) => { scope.done() }) +test('getCodeOwnersFile network error', async (t) => { + const options = { + owner: 'nodejs', + repo: 'node-auto-test-6', + prId: 12347, + logger: { info: () => {}, error: () => {}, debug: () => {}, child: function () { return this } }, + retries: 1, + defaultBranch: 'main', + retryInterval: 10 + } + + const base = 'https://localhost' + const filePath = '/CODEOWNERS' + const url = `${base}${filePath}` + const scope = nock(base) + .get(filePath) + .replyWithError('socket hang up') + + await t.assert.rejects(getCodeOwnersFile(url, options), { message: 'socket hang up' }) + scope.done() +}) + test('pingOwners success', async (t) => { const options = { owner: 'nodejs',