From 2c2034b4a0cff35efdb873e0f39e16804d5e11e4 Mon Sep 17 00:00:00 2001 From: NopeCharon <115794254+NopeCharon@users.noreply.github.com> Date: Sat, 24 Oct 2026 12:26:14 +0530 Subject: [PATCH] fix: don't crash when posting the CI link comment fails pushStarted() posts a "CI: " (or "V8 CI:" / "Lite-CI:") comment on the pull request when Jenkins reports a pending build, but called createPrComment() without awaiting it or attaching a rejection handler. createPrComment() logs the error and rethrows it, so any GitHub API failure while commenting (5xx, rate limiting, locked PR, timeout) turned into an unhandled promise rejection. server.js treats those as fatal and exits the process, taking down the bot because of a best-effort comment. Route the three comment calls through a small postCiComment() helper that swallows the already-logged rejection, so the commit status update and the rest of the bot keep working. Add a unit test which makes the comment endpoint return 500 and asserts that the status is still created and no unhandled rejection is raised; it fails without this change. Fixes: https://github.com/nodejs/github-bot/issues/704 Signed-off-by: NopeCharon <115794254+NopeCharon@users.noreply.github.com> --- lib/push-jenkins-update.js | 12 +++++++++--- test/unit/push-jenkins-update.test.js | 26 +++++++++++++++++++++++++- 2 files changed, 34 insertions(+), 4 deletions(-) diff --git a/lib/push-jenkins-update.js b/lib/push-jenkins-update.js index a2ec9cde..61b3cdb6 100644 --- a/lib/push-jenkins-update.js +++ b/lib/push-jenkins-update.js @@ -13,15 +13,15 @@ export function pushStarted (options, build, cb) { if (build.status === 'pending') { switch (build.identifier) { case 'node-test-pull-request': - createPrComment(Object.assign({ issue_number: pr }, options), `CI: ${build.url}`) + postCiComment(options, pr, `CI: ${build.url}`) break case 'node-test-commit-v8-linux': - createPrComment(Object.assign({ issue_number: pr }, options), `V8 CI: ${build.url}`) + postCiComment(options, pr, `V8 CI: ${build.url}`) break case 'node-test-pull-request-lite-pipeline': - createPrComment(Object.assign({ issue_number: pr }, options), `Lite-CI: ${build.url}`) + postCiComment(options, pr, `Lite-CI: ${build.url}`) break default: @@ -70,6 +70,12 @@ export function pushEnded (options, build, cb) { }) } +function postCiComment (options, pr, body) { + // posting the CI link is best effort: createPrComment() has already logged any error, + // and letting it surface as an unhandled rejection would terminate the bot process + createPrComment(Object.assign({ issue_number: pr }, options), body).catch(() => {}) +} + function findPrInRef (gitRef) { // refs/pull/12345/head return parseInt(gitRef.split('/')[2], 10) diff --git a/test/unit/push-jenkins-update.test.js b/test/unit/push-jenkins-update.test.js index 0ef3f3f8..5858bdb2 100644 --- a/test/unit/push-jenkins-update.test.js +++ b/test/unit/push-jenkins-update.test.js @@ -1,6 +1,6 @@ import test from 'node:test' -import { findLatestCommitInPr } from '../../lib/push-jenkins-update.js' +import { findLatestCommitInPr, pushStarted } from '../../lib/push-jenkins-update.js' import fetchMock from 'fetch-mock' import readFixture from '../read-fixture.js' @@ -65,3 +65,27 @@ test('findLatestCommitInPr: paginates results when more than 100 commits in a PR thirdPageScope.callHistory.called() fourthPageScope.callHistory.called() }) + +test('pushStarted: updates the commit status even when posting the CI comment fails', (t, done) => { + const build = readFixture('jenkins-test-pull-request-success-payload.json') + build.status = 'pending' + const logger = { info: () => {}, error: () => {}, debug: () => {}, child: function () { return this } } + + fetchMock.mockGlobal() + + const commentUrl = 'https://api.github.com/repos/nodejs/node/issues/12345/comments' + fetchMock.route({ url: commentUrl, method: 'POST' }, 500) + fetchMock.route('https://api.github.com/repos/nodejs/node/pulls/12345/commits', readFixture('pr-commits.json')) + const statusUrl = 'https://api.github.com/repos/nodejs/node/statuses/8a5fec2a6bade91e544a30314d7cf21f8a200de1' + fetchMock.route({ url: statusUrl, method: 'POST' }, 201) + + t.plan(3) + + pushStarted({ owner: 'nodejs', repo: 'node', logger }, build, (err) => { + t.assert.strictEqual(err, undefined) + t.assert.strictEqual(fetchMock.callHistory.called(commentUrl), true) + t.assert.strictEqual(fetchMock.callHistory.called(statusUrl), true) + // give a rejected comment promise the chance to surface as an unhandled rejection + setImmediate(done) + }) +})