diff --git a/lib/manager/npm/post-update/index.js b/lib/manager/npm/post-update/index.js index 00634f173f35fed99a16550fdf7b4b84f8252dcb..0f651a890965dc854ff238b9262f752c6f33312c 100644 --- a/lib/manager/npm/post-update/index.js +++ b/lib/manager/npm/post-update/index.js @@ -328,11 +328,10 @@ async function getAdditionalFiles(config, packageFiles) { logger.info({ stderr: res.stderr }, 'Host key verification failed'); throw new Error('internal-error'); } - const err = Error('lockfile-error'); - err.details = res.stderr; - err.fileName = lockFile; - err.releaseTimestamp = config.releaseTimestamp; - throw err; + lockFileErrors.push({ + lockFile, + stderr: res.stderr, + }); } else { const existingContent = await platform.getFile( lockFile, @@ -382,11 +381,10 @@ async function getAdditionalFiles(config, packageFiles) { logger.info({ stderr: res.stderr }, 'Host key verification failed'); throw new Error('internal-error'); } - const err = Error('lockfile-error'); - err.details = res.stderr; - err.fileName = lockFile; - err.releaseTimestamp = config.releaseTimestamp; - throw err; + lockFileErrors.push({ + lockFile, + stderr: res.stderr, + }); } else { const existingContent = await platform.getFile( lockFileName, @@ -433,11 +431,10 @@ async function getAdditionalFiles(config, packageFiles) { logger.info({ stdout: res.stdout }, 'Host key verification failed'); throw new Error('internal-error'); } - const err = Error('lockfile-error'); - err.details = res.stdout; - err.fileName = lockFile; - err.releaseTimestamp = config.releaseTimestamp; - throw err; + lockFileErrors.push({ + lockFile, + stderr: res.stderr, + }); } else { const existingContent = await platform.getFile( lockFile, @@ -515,11 +512,10 @@ async function getAdditionalFiles(config, packageFiles) { throw new Error('registry-failure'); } } - const err = Error('lockfile-error'); - err.details = res.stderr; - err.fileName = lockFile; - err.releaseTimestamp = config.releaseTimestamp; - throw err; + lockFileErrors.push({ + lockFile, + stderr: res.stderr, + }); } else { for (const packageFile of packageFiles.npm) { const baseDir = path.dirname(packageFile.packageFile); diff --git a/lib/workers/branch/index.js b/lib/workers/branch/index.js index d1a6cff02d94878d9b0a9b0c4145527b57274ca4..d0cc407158d4ebcb7b3434403a31eaa5966ca15e 100644 --- a/lib/workers/branch/index.js +++ b/lib/workers/branch/index.js @@ -1,3 +1,5 @@ +const { DateTime } = require('luxon'); + const schedule = require('./schedule'); const { getUpdatedPackageFiles } = require('./get-updated'); const { getAdditionalFiles } = require('../../manager/npm/post-update'); @@ -153,6 +155,24 @@ async function processBranch(branchConfig, packageFiles) { } else { logger.debug('No updated lock files in branch'); } + if (config.lockFileErrors && config.lockFileErrors.length) { + if (config.releaseTimestamp) { + logger.debug(`Branch timestamp: ` + config.releaseTimestamp); + const releaseTimestamp = DateTime.fromISO(config.releaseTimestamp); + if (releaseTimestamp.plus({ days: 1 }) < DateTime.local()) { + logger.info('PR is older than a day, raise PR with lock file errors'); + } else if (branchExists) { + logger.info( + 'PR is less than a day old but branchExists so updating anyway' + ); + } else { + logger.info('PR is less than a day old - raise error instead of PR'); + throw new Error('lockfile-error'); + } + } else { + logger.debug('PR has no releaseTimestamp'); + } + } const committedFiles = await commitFilesToBranch(config); // istanbul ignore if @@ -207,7 +227,7 @@ async function processBranch(branchConfig, packageFiles) { throw err; } if (err.message === 'lockfile-error') { - logger.info('Lock file error'); + logger.debug('Passing lockfile-error up'); throw err; } if (err.message !== 'registry-failure') { @@ -226,7 +246,7 @@ async function processBranch(branchConfig, packageFiles) { const pr = await prWorker.ensurePr(config); // TODO: ensurePr should check for automerge itself if (pr) { - const topic = 'Lock file problem'; + const topic = ':warning: Lock file problem'; if (config.lockFileErrors && config.lockFileErrors.length) { logger.warn( { lockFileErrors: config.lockFileErrors }, @@ -236,7 +256,7 @@ async function processBranch(branchConfig, packageFiles) { content += config.lockFileErrors.length > 1 ? 'lock files' : 'a lock file'; content += - '. This is usually happens when you have private modules but have not added configuration for [private module support](https://renovatebot.com/docs/deep-dives/private-modules). It is strongly recommended that you do not merge this PR as-is.'; + '. The most frequent cause is when you have private modules but have not added configuration for [private module support](https://renovatebot.com/docs/private-modules/). It is strongly recommended that you do not merge this PR as-is.'; content += '\n\nRenovate **will not retry** generating a lockfile for this PR unless either (a) the `package.json` in this branch needs updating, or (b) '; if (config.recreateClosed) { @@ -246,12 +266,29 @@ async function processBranch(branchConfig, packageFiles) { content += 'you rename then delete this PR unmerged, so that it can be regenerated.'; } - content += '\n\nThe output from `stderr` is included below:\n\n'; + content += '\n\nThe lock file failure details are included below:\n\n'; config.lockFileErrors.forEach(error => { content += `##### ${error.lockFile}\n\n`; content += `\`\`\`\n${error.stderr}\n\`\`\`\n\n`; }); await platform.ensureComment(pr.number, topic, content); + const context = 'renovate/lock-files'; + const description = 'Lock file update failure'; + const state = 'failure'; + const existingState = await platform.getBranchStatusCheck( + config.branchName, + context + ); + // Check if state needs setting + if (existingState !== state) { + logger.debug(`Updating status check state to failed`); + await platform.setBranchStatus( + config.branchName, + context, + description, + state + ); + } } else { if (config.updatedLockFiles && config.updatedLockFiles.length) { await platform.ensureCommentRemoval(pr.number, topic); diff --git a/lib/workers/repository/error-lockfile.js b/lib/workers/repository/error-lockfile.js deleted file mode 100644 index a486dfa25752c080c7cca81c75ddf98e174ed07a..0000000000000000000000000000000000000000 --- a/lib/workers/repository/error-lockfile.js +++ /dev/null @@ -1,50 +0,0 @@ -const { DateTime } = require('luxon'); - -module.exports = { - raiseLockFileIssue, -}; - -function raiseLockFileIssue(err) { - logger.debug('raiseLockFileIssue()'); - if (!err.releaseTimestamp) { - logger.warn('lock file error without release timestamp'); - return; - } - let body = - 'Renovate encountered an error when updating a lock file, and this may need your manual intervention to resolve. '; - body += - "It's possible that this lock file problem could prevent most or all PRs from being created. "; - body += - 'Renovate will close this issue automatically if it no longer experiences any lock file updating errors (such as if this was a temporary error).\n\n'; - body += - 'The most common cause for failed lock file updating is the use of private npm modules or a private npm server while Renovate has been given no credentials to download them. '; - body += - "If this is the case, then please check out Renovate's [private module support documentation](https://renovatebot.com/docs/private-modules/).\n\n\n"; - body += - 'If you just want this error to go away, the simplest options are either:\n'; - body += ' - Delete any lockfiles you do not care about, or \n'; - body += - ' - Add setting `updateLockFiles=false` to your renovate config and Renovate will skip lock files in PRs. This is not recommended in most circumstances because then your lock file is invalidated.\n\n'; - body += `File name: \`${err.fileName}\`\n\n`; - body += `Details:\n\n`; - body += '```\n'; - const details = (err.details || '') - .replace(/npm ERR! A complete log of this run can be found in.*\n.*/g, '') - .replace(/\n+$/, ''); - body += `${details}\n`; - body += '```\n'; - const releaseTimestamp = DateTime.fromISO(err.releaseTimestamp); - if (releaseTimestamp.plus({ days: 1 }) < DateTime.local()) { - /* - const res = await platform.ensureIssue( - 'Action Required: Fix Renovate Configuration', - body - ); - logger.info('Lock file warning issue: ' + res); - */ - logger.warn( - { body }, - 'Failed lockfile generation for release over one day old' - ); - } -} diff --git a/lib/workers/repository/error.js b/lib/workers/repository/error.js index c986346427e1b910e0334f4c6328bd9a832a7d22..79ea22aad3b52dc63a2abb80f8f1593e23e38cb8 100644 --- a/lib/workers/repository/error.js +++ b/lib/workers/repository/error.js @@ -1,5 +1,4 @@ const { raiseConfigWarningIssue } = require('./error-config'); -const { raiseLockFileIssue } = require('./error-lockfile'); module.exports = { handleError, @@ -74,7 +73,8 @@ async function handleError(config, err) { } if (err.message === 'lockfile-error') { delete config.branchList; // eslint-disable-line no-param-reassign - await raiseLockFileIssue(err); + logger.info('Lock file error - aborting'); + delete config.branchList; // eslint-disable-line no-param-reassign return err.message; } // Swallow this error so that other repositories can be processed diff --git a/test/workers/branch/index.spec.js b/test/workers/branch/index.spec.js index 5b2d1ad0c567a3ab26c3836534a4d7378bf90df9..7acadcf3da5e7e3014c508e17f9e09b5772f5f96 100644 --- a/test/workers/branch/index.spec.js +++ b/test/workers/branch/index.spec.js @@ -188,7 +188,7 @@ describe('workers/branch', () => { expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(1); expect(prWorker.checkAutoMerge.mock.calls).toHaveLength(1); }); - it('ensures PR and adds lock file error comment', async () => { + it('ensures PR and adds lock file error comment if no releaseTimestamp', async () => { getUpdated.getUpdatedPackageFiles.mockReturnValueOnce({ updatedPackageFiles: [{}], }); @@ -207,6 +207,64 @@ describe('workers/branch', () => { expect(prWorker.ensurePr.mock.calls).toHaveLength(1); expect(prWorker.checkAutoMerge.mock.calls).toHaveLength(0); }); + it('ensures PR and adds lock file error comment if old releaseTimestamp', async () => { + getUpdated.getUpdatedPackageFiles.mockReturnValueOnce({ + updatedPackageFiles: [{}], + }); + npmPostExtract.getAdditionalFiles.mockReturnValueOnce({ + lockFileError: false, + updatedLockFiles: [{}], + }); + platform.branchExists.mockReturnValueOnce(true); + automerge.tryBranchAutomerge.mockReturnValueOnce('failed'); + prWorker.ensurePr.mockReturnValueOnce({}); + prWorker.checkAutoMerge.mockReturnValueOnce(true); + config.lockFileErrors = [{}]; + config.releaseTimestamp = '2018-04-26T05:15:51.877Z'; + await branchWorker.processBranch(config); + expect(platform.ensureComment.mock.calls).toHaveLength(1); + expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(0); + expect(prWorker.ensurePr.mock.calls).toHaveLength(1); + expect(prWorker.checkAutoMerge.mock.calls).toHaveLength(0); + }); + it('ensures PR and adds lock file error comment if new releaseTimestamp and branch exists', async () => { + getUpdated.getUpdatedPackageFiles.mockReturnValueOnce({ + updatedPackageFiles: [{}], + }); + npmPostExtract.getAdditionalFiles.mockReturnValueOnce({ + lockFileError: false, + updatedLockFiles: [{}], + }); + platform.branchExists.mockReturnValueOnce(true); + automerge.tryBranchAutomerge.mockReturnValueOnce('failed'); + prWorker.ensurePr.mockReturnValueOnce({}); + prWorker.checkAutoMerge.mockReturnValueOnce(true); + config.lockFileErrors = [{}]; + config.releaseTimestamp = new Date().toISOString(); + await branchWorker.processBranch(config); + expect(platform.ensureComment.mock.calls).toHaveLength(1); + expect(platform.ensureCommentRemoval.mock.calls).toHaveLength(0); + expect(prWorker.ensurePr.mock.calls).toHaveLength(1); + expect(prWorker.checkAutoMerge.mock.calls).toHaveLength(0); + }); + it('throws error if lock file errors and new releaseTimestamp', () => { + getUpdated.getUpdatedPackageFiles.mockReturnValueOnce({ + updatedPackageFiles: [{}], + }); + npmPostExtract.getAdditionalFiles.mockReturnValueOnce({ + lockFileError: false, + updatedLockFiles: [{}], + }); + platform.branchExists.mockReturnValueOnce(false); + automerge.tryBranchAutomerge.mockReturnValueOnce('failed'); + prWorker.ensurePr.mockReturnValueOnce({}); + prWorker.checkAutoMerge.mockReturnValueOnce(true); + config.lockFileErrors = [{}]; + config.releaseTimestamp = new Date().toISOString(); + expect(branchWorker.processBranch(config)).rejects.toThrow( + 'lockfile-error' + ); + }); it('ensures PR and adds lock file error comment recreate closed', async () => { getUpdated.getUpdatedPackageFiles.mockReturnValueOnce({ updatedPackageFiles: [{}], diff --git a/test/workers/repository/error-lockfile.spec.js b/test/workers/repository/error-lockfile.spec.js deleted file mode 100644 index 3f62ccee0bae0eea0c3bbccd438e1e74efeaa9f4..0000000000000000000000000000000000000000 --- a/test/workers/repository/error-lockfile.spec.js +++ /dev/null @@ -1,29 +0,0 @@ -const { DateTime } = require('luxon'); -const { - raiseLockFileIssue, -} = require('../../../lib/workers/repository/error-lockfile'); - -describe('workers/repository/error-config', () => { - describe('raiseLockFileIssue()', () => { - it('warns if no releaseTimestamp', async () => { - const error = new Error('lockfile-error'); - error.fileName = 'yarn.lock'; - error.details = 'Some details here'; - await raiseLockFileIssue(error); - }); - it('logs warning if greater than a day old', async () => { - const error = new Error('lockfile-error'); - error.fileName = 'yarn.lock'; - error.details = 'Some details here'; - error.releaseTimestamp = '2017-02-06T20:01:41+00:00'; - await raiseLockFileIssue(error); - }); - it('logs no warning if less than a day old', async () => { - const error = new Error('lockfile-error'); - error.fileName = 'yarn.lock'; - error.details = 'Some details here'; - error.releaseTimestamp = DateTime.local().toISO(); - await raiseLockFileIssue(error); - }); - }); -});