From be926de9b1a6bc8952935b97058c8d18e99fe4d0 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Wed, 17 May 2023 16:59:21 +0300 Subject: [PATCH 1/3] rename error-config.ts & error-config.spec.ts --- .../repository/{error-config.spec.ts => error-issue.spec.ts} | 4 ++-- lib/workers/repository/{error-config.ts => error-issue.ts} | 0 lib/workers/repository/error.spec.ts | 2 +- lib/workers/repository/error.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename lib/workers/repository/{error-config.spec.ts => error-issue.spec.ts} (96%) rename lib/workers/repository/{error-config.ts => error-issue.ts} (100%) diff --git a/lib/workers/repository/error-config.spec.ts b/lib/workers/repository/error-issue.spec.ts similarity index 96% rename from lib/workers/repository/error-config.spec.ts rename to lib/workers/repository/error-issue.spec.ts index 5076984e35673d..fd5eaccfe1df6b 100644 --- a/lib/workers/repository/error-config.spec.ts +++ b/lib/workers/repository/error-issue.spec.ts @@ -3,7 +3,7 @@ import { RenovateConfig, getConfig, platform } from '../../../test/util'; import { GlobalConfig } from '../../config/global'; import { CONFIG_VALIDATION } from '../../constants/error-messages'; import type { Pr } from '../../modules/platform'; -import { raiseConfigWarningIssue } from './error-config'; +import { raiseConfigWarningIssue } from './error-issue'; jest.mock('../../modules/platform'); @@ -14,7 +14,7 @@ beforeEach(() => { config = getConfig(); }); -describe('workers/repository/error-config', () => { +describe('workers/repository/error-issue', () => { describe('raiseConfigWarningIssue()', () => { beforeEach(() => { GlobalConfig.reset(); diff --git a/lib/workers/repository/error-config.ts b/lib/workers/repository/error-issue.ts similarity index 100% rename from lib/workers/repository/error-config.ts rename to lib/workers/repository/error-issue.ts diff --git a/lib/workers/repository/error.spec.ts b/lib/workers/repository/error.spec.ts index 891dd33e2b722a..2383653522f5b2 100644 --- a/lib/workers/repository/error.spec.ts +++ b/lib/workers/repository/error.spec.ts @@ -30,7 +30,7 @@ import { import { ExternalHostError } from '../../types/errors/external-host-error'; import handleError from './error'; -jest.mock('./error-config'); +jest.mock('./error-issue'); let config: RenovateConfig; diff --git a/lib/workers/repository/error.ts b/lib/workers/repository/error.ts index 75178913142937..35ce328e4f013d 100644 --- a/lib/workers/repository/error.ts +++ b/lib/workers/repository/error.ts @@ -33,7 +33,7 @@ import { } from '../../constants/error-messages'; import { logger } from '../../logger'; import { ExternalHostError } from '../../types/errors/external-host-error'; -import { raiseConfigWarningIssue } from './error-config'; +import { raiseConfigWarningIssue } from './error-issue'; export default async function handleError( config: RenovateConfig, From 493e3195e168eebe0f5ad4c81e4e0fc933c18fa4 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Wed, 17 May 2023 17:02:20 +0300 Subject: [PATCH 2/3] generalize error issue creation --- lib/workers/repository/error-issue.spec.ts | 49 ++++++++-- lib/workers/repository/error-issue.ts | 103 ++++++++++++++------- 2 files changed, 110 insertions(+), 42 deletions(-) diff --git a/lib/workers/repository/error-issue.spec.ts b/lib/workers/repository/error-issue.spec.ts index fd5eaccfe1df6b..b2d725c042d5df 100644 --- a/lib/workers/repository/error-issue.spec.ts +++ b/lib/workers/repository/error-issue.spec.ts @@ -1,7 +1,13 @@ import { mock } from 'jest-mock-extended'; -import { RenovateConfig, getConfig, platform } from '../../../test/util'; +import { + RenovateConfig, + getConfig, + partial, + platform, +} from '../../../test/util'; import { GlobalConfig } from '../../config/global'; import { CONFIG_VALIDATION } from '../../constants/error-messages'; +import { logger } from '../../logger'; import type { Pr } from '../../modules/platform'; import { raiseConfigWarningIssue } from './error-issue'; @@ -24,9 +30,16 @@ describe('workers/repository/error-issue', () => { const error = new Error(CONFIG_VALIDATION); error.validationSource = 'package.json'; error.validationMessage = 'some-message'; + error.validationError = 'some-error'; platform.ensureIssue.mockResolvedValueOnce('created'); + const res = await raiseConfigWarningIssue(config, error); + expect(res).toBeUndefined(); + expect(logger.warn).toHaveBeenCalledWith( + { configError: error, res: 'created' }, + 'Configuration Warning' + ); }); it('creates issues (dryRun)', async () => { @@ -35,50 +48,74 @@ describe('workers/repository/error-issue', () => { error.validationMessage = 'some-message'; platform.ensureIssue.mockResolvedValueOnce('created'); GlobalConfig.set({ dryRun: 'full' }); + const res = await raiseConfigWarningIssue(config, error); + expect(res).toBeUndefined(); + expect(logger.info).toHaveBeenCalledWith( + { configError: error }, + 'DRY-RUN: Would ensure configuration error issue' + ); }); it('handles onboarding', async () => { const error = new Error(CONFIG_VALIDATION); error.validationSource = 'package.json'; error.validationMessage = 'some-message'; - platform.getBranchPr.mockResolvedValue({ - ...mock(), + const pr = partial({ + title: 'onboarding', number: 1, state: 'open', }); + platform.getBranchPr.mockResolvedValue(pr); + const res = await raiseConfigWarningIssue(config, error); + expect(res).toBeUndefined(); + expect(platform.updatePr).toHaveBeenCalledWith( + expect.objectContaining({ prTitle: pr.title, number: pr.number }) + ); }); it('handles onboarding (dryRun)', async () => { const error = new Error(CONFIG_VALIDATION); error.validationSource = 'package.json'; error.validationMessage = 'some-message'; - platform.getBranchPr.mockResolvedValue({ - ...mock(), + const pr = partial({ number: 1, state: 'open', }); + platform.getBranchPr.mockResolvedValue(pr); GlobalConfig.set({ dryRun: 'full' }); + const res = await raiseConfigWarningIssue(config, error); + expect(res).toBeUndefined(); + expect(logger.info).toHaveBeenCalledWith( + `DRY-RUN: Would update PR #${pr.number}` + ); }); it('disable issue creation on config failure', async () => { + const notificationName = 'configErrorIssue'; const error = new Error(CONFIG_VALIDATION); error.validationSource = 'package.json'; error.validationMessage = 'some-message'; // config.suppressNotifications = ['deprecationWarningIssues'] - config.suppressNotifications = ['configErrorIssue']; + config.suppressNotifications = [notificationName]; platform.getBranchPr.mockResolvedValueOnce({ ...mock(), number: 1, state: '!open', }); + const res = await raiseConfigWarningIssue(config, error); + expect(res).toBeUndefined(); + expect(logger.info).toHaveBeenCalledWith( + { notificationName }, + 'Configuration failure, issues will be suppressed' + ); }); }); }); diff --git a/lib/workers/repository/error-issue.ts b/lib/workers/repository/error-issue.ts index 0fba9c00eefbbf..54a6d659e00f5c 100644 --- a/lib/workers/repository/error-issue.ts +++ b/lib/workers/repository/error-issue.ts @@ -2,61 +2,92 @@ import { GlobalConfig } from '../../config/global'; import type { RenovateConfig } from '../../config/types'; import { logger } from '../../logger'; -import { platform } from '../../modules/platform'; +import { Pr, platform } from '../../modules/platform'; import { regEx } from '../../util/regex'; -export async function raiseConfigWarningIssue( +export function raiseConfigWarningIssue( config: RenovateConfig, error: Error ): Promise { logger.debug('raiseConfigWarningIssue()'); - let body = `There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.\n\n`; + const title = `Action Required: Fix Renovate Configuration`; + const body = `There is an error with this repository's Renovate configuration that needs to be fixed. As a precaution, Renovate will stop PRs until it is resolved.\n\n`; + const notificationName = 'configErrorIssue'; + return raiseWarningIssue(config, notificationName, title, body, error); +} + +async function raiseWarningIssue( + config: RenovateConfig, + notificationName: string, + title: string, + initialBody: string, + error: Error +): Promise { + let body = initialBody; if (error.validationSource) { body += `Location: \`${error.validationSource}\`\n`; } - body += `Error type: ${error.validationError!}\n`; + if (error.validationError) { + body += `Error type: ${error.validationError}\n`; + } if (error.validationMessage) { body += `Message: \`${error.validationMessage.replace( regEx(/`/g), "'" )}\`\n`; } + const pr = await platform.getBranchPr(config.onboardingBranch!); if (pr?.state === 'open') { - logger.debug('Updating onboarding PR with config error notice'); - body = `## Action Required: Fix Renovate Configuration\n\n${body}`; - body += `\n\nOnce you have resolved this problem (in this onboarding branch), Renovate will return to providing you with a preview of your repository's configuration.`; - if (GlobalConfig.get('dryRun')) { - logger.info(`DRY-RUN: Would update PR #${pr.number}`); - } else { - try { - await platform.updatePr({ - number: pr.number, - prTitle: config.onboardingPrTitle!, - prBody: body, - }); - } catch (err) /* istanbul ignore next */ { - logger.warn({ err }, 'Error updating onboarding PR'); - } - } - } else if (GlobalConfig.get('dryRun')) { - logger.info('DRY-RUN: Would ensure config error issue'); - } else if (config.suppressNotifications?.includes('configErrorIssue')) { + await handleOnboardingPr(pr, body); + return; + } + + if (GlobalConfig.get('dryRun')) { logger.info( - 'configErrorIssue - configuration failure, issues will be suppressed' + { configError: error }, + 'DRY-RUN: Would ensure configuration error issue' ); - } else { - const once = false; - const shouldReopen = config.configWarningReuseIssue; - const res = await platform.ensureIssue({ - title: `Action Required: Fix Renovate Configuration`, - body, - once, - shouldReOpen: shouldReopen, - confidential: config.confidential, + return; + } + + if (config.suppressNotifications?.includes(notificationName)) { + logger.info( + { notificationName }, + 'Configuration failure, issues will be suppressed' + ); + return; + } + + const res = await platform.ensureIssue({ + title, + body, + once: false, + shouldReOpen: config.configWarningReuseIssue, + confidential: config.confidential, + }); + if (res === 'created') { + logger.warn({ configError: error, res }, 'Configuration Warning'); + } +} + +async function handleOnboardingPr(pr: Pr, issueMessage: string): Promise { + logger.debug('Updating onboarding PR with config error notice'); + if (GlobalConfig.get('dryRun')) { + logger.info(`DRY-RUN: Would update PR #${pr.number}`); + return; + } + + let prBody = `## Action Required: Fix Renovate Configuration\n\n${issueMessage}`; + prBody += `\n\nOnce you have resolved this problem (in this onboarding branch), Renovate will return to providing you with a preview of your repository's configuration.`; + + try { + await platform.updatePr({ + number: pr.number, + prTitle: pr.title, + prBody, }); - if (res === 'created') { - logger.warn({ configError: error, res }, 'Config Warning'); - } + } catch (err) /* istanbul ignore next */ { + logger.warn({ err }, 'Error updating onboarding PR'); } } From a67f934be678347bc48156542303857ca175bab5 Mon Sep 17 00:00:00 2001 From: Gabriel-Ladzaretti Date: Wed, 17 May 2023 17:07:42 +0300 Subject: [PATCH 3/3] Revert "rename error-config.ts & error-config.spec.ts" This reverts commit be926de9b1a6bc8952935b97058c8d18e99fe4d0. --- .../repository/{error-issue.spec.ts => error-config.spec.ts} | 4 ++-- lib/workers/repository/{error-issue.ts => error-config.ts} | 0 lib/workers/repository/error.spec.ts | 2 +- lib/workers/repository/error.ts | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) rename lib/workers/repository/{error-issue.spec.ts => error-config.spec.ts} (97%) rename lib/workers/repository/{error-issue.ts => error-config.ts} (100%) diff --git a/lib/workers/repository/error-issue.spec.ts b/lib/workers/repository/error-config.spec.ts similarity index 97% rename from lib/workers/repository/error-issue.spec.ts rename to lib/workers/repository/error-config.spec.ts index b2d725c042d5df..109bc7aa7d374f 100644 --- a/lib/workers/repository/error-issue.spec.ts +++ b/lib/workers/repository/error-config.spec.ts @@ -9,7 +9,7 @@ import { GlobalConfig } from '../../config/global'; import { CONFIG_VALIDATION } from '../../constants/error-messages'; import { logger } from '../../logger'; import type { Pr } from '../../modules/platform'; -import { raiseConfigWarningIssue } from './error-issue'; +import { raiseConfigWarningIssue } from './error-config'; jest.mock('../../modules/platform'); @@ -20,7 +20,7 @@ beforeEach(() => { config = getConfig(); }); -describe('workers/repository/error-issue', () => { +describe('workers/repository/error-config', () => { describe('raiseConfigWarningIssue()', () => { beforeEach(() => { GlobalConfig.reset(); diff --git a/lib/workers/repository/error-issue.ts b/lib/workers/repository/error-config.ts similarity index 100% rename from lib/workers/repository/error-issue.ts rename to lib/workers/repository/error-config.ts diff --git a/lib/workers/repository/error.spec.ts b/lib/workers/repository/error.spec.ts index 2383653522f5b2..891dd33e2b722a 100644 --- a/lib/workers/repository/error.spec.ts +++ b/lib/workers/repository/error.spec.ts @@ -30,7 +30,7 @@ import { import { ExternalHostError } from '../../types/errors/external-host-error'; import handleError from './error'; -jest.mock('./error-issue'); +jest.mock('./error-config'); let config: RenovateConfig; diff --git a/lib/workers/repository/error.ts b/lib/workers/repository/error.ts index 35ce328e4f013d..75178913142937 100644 --- a/lib/workers/repository/error.ts +++ b/lib/workers/repository/error.ts @@ -33,7 +33,7 @@ import { } from '../../constants/error-messages'; import { logger } from '../../logger'; import { ExternalHostError } from '../../types/errors/external-host-error'; -import { raiseConfigWarningIssue } from './error-issue'; +import { raiseConfigWarningIssue } from './error-config'; export default async function handleError( config: RenovateConfig,