From a3043c47ec63f7bff5611589e0949f9bf8bb2fd6 Mon Sep 17 00:00:00 2001 From: Rhys Arkins <rhys@arkins.net> Date: Thu, 28 Jun 2018 10:17:17 +0200 Subject: [PATCH] feat: log warning if lock file error persists for 1 day Raises an additional log file warning whenever lock file errors persist for a day or longer. The idea of this is that temporary errors - e.g. caused by npmjs itself - should not disturb the user. 1 day seems like a reasonable time for multiple attemps to be made first, assuming it has been scheduled. Once this is tested in production for a little while and no unexpected problems, it will be converted to actually raise a config warning issue in the repo to get user attention. --- lib/manager/npm/post-update/index.js | 24 +++++++-- lib/workers/repository/error-lockfile.js | 50 +++++++++++++++++++ lib/workers/repository/error.js | 2 + .../workers/repository/error-lockfile.spec.js | 29 +++++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 lib/workers/repository/error-lockfile.js create mode 100644 test/workers/repository/error-lockfile.spec.js diff --git a/lib/manager/npm/post-update/index.js b/lib/manager/npm/post-update/index.js index 27d3caed1b..8da96c9e3c 100644 --- a/lib/manager/npm/post-update/index.js +++ b/lib/manager/npm/post-update/index.js @@ -331,7 +331,11 @@ async function getAdditionalFiles(config, packageFiles) { logger.info({ stderr: res.stderr }, 'Host key verification failed'); throw new Error('internal-error'); } - throw new Error('lockfile-error'); + const err = Error('lockfile-error'); + err.details = res.stderr; + err.fileName = lockFile; + err.releaseTimestamp = config.releaseTimestamp; + throw err; } else { const existingContent = await platform.getFile( lockFile, @@ -381,7 +385,11 @@ async function getAdditionalFiles(config, packageFiles) { logger.info({ stderr: res.stderr }, 'Host key verification failed'); throw new Error('internal-error'); } - throw new Error('lockfile-error'); + const err = Error('lockfile-error'); + err.details = res.stderr; + err.fileName = lockFile; + err.releaseTimestamp = config.releaseTimestamp; + throw err; } else { const existingContent = await platform.getFile( lockFileName, @@ -428,7 +436,11 @@ async function getAdditionalFiles(config, packageFiles) { logger.info({ stdout: res.stdout }, 'Host key verification failed'); throw new Error('internal-error'); } - throw new Error('lockfile-error'); + const err = Error('lockfile-error'); + err.details = res.stdout; + err.fileName = lockFile; + err.releaseTimestamp = config.releaseTimestamp; + throw err; } else { const existingContent = await platform.getFile( lockFile, @@ -502,7 +514,11 @@ async function getAdditionalFiles(config, packageFiles) { throw new Error('registry-failure'); } } - throw new Error('lockfile-error'); + const err = Error('lockfile-error'); + err.details = res.stderr; + err.fileName = lockFile; + err.releaseTimestamp = config.releaseTimestamp; + throw err; } else { for (const packageFile of packageFiles.npm) { const baseDir = path.dirname(packageFile.packageFile); diff --git a/lib/workers/repository/error-lockfile.js b/lib/workers/repository/error-lockfile.js new file mode 100644 index 0000000000..a486dfa257 --- /dev/null +++ b/lib/workers/repository/error-lockfile.js @@ -0,0 +1,50 @@ +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 01200cca13..fc0e122284 100644 --- a/lib/workers/repository/error.js +++ b/lib/workers/repository/error.js @@ -1,4 +1,5 @@ const { raiseConfigWarningIssue } = require('./error-config'); +const { raiseLockFileIssue } = require('./error-lockfile'); module.exports = { handleError, @@ -57,6 +58,7 @@ async function handleError(config, err) { return err.message; } else if (err.message === 'lockfile-error') { delete config.branchList; // eslint-disable-line no-param-reassign + await raiseLockFileIssue(err); return err.message; } // Swallow this error so that other repositories can be processed diff --git a/test/workers/repository/error-lockfile.spec.js b/test/workers/repository/error-lockfile.spec.js new file mode 100644 index 0000000000..3f62ccee0b --- /dev/null +++ b/test/workers/repository/error-lockfile.spec.js @@ -0,0 +1,29 @@ +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); + }); + }); +}); -- GitLab