From f0cd0cb8b8d48c4a627c7e504a182e9f6bc7d41d Mon Sep 17 00:00:00 2001
From: Rhys Arkins <rhys@arkins.net>
Date: Thu, 22 Mar 2018 11:55:58 +0100
Subject: [PATCH] feat: raise config action issue if failing to look up locked
dependency (#1704)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
If an npm dependency can’t be found, and the package.json has a lock file, then Renovate will encounter lock file errors every time *any* dependency in that package.json has an update. Instead of raising PRs with an error, we instead now stop raising PRs and instead raise a config warning issue. Users can “dismiss†this by setting config option `updateLockFiles` to false.
Closes #1697
---
lib/manager/index.js | 1 -
lib/manager/npm/package.js | 21 ++++++++++------
lib/workers/repository/error-config.js | 6 ++---
lib/workers/repository/updates/index.js | 1 +
.../npm/__snapshots__/package.spec.js.snap | 22 +---------------
test/manager/npm/package.spec.js | 25 +++++++++++--------
6 files changed, 34 insertions(+), 42 deletions(-)
diff --git a/lib/manager/index.js b/lib/manager/index.js
index 7f7a753de4..927ba8f175 100644
--- a/lib/manager/index.js
+++ b/lib/manager/index.js
@@ -191,6 +191,5 @@ async function resolvePackageFiles(config) {
return packageFile;
});
- platform.ensureIssueClosing('Action Required: Fix Renovate Configuration');
return checkMonorepos({ ...config, packageFiles });
}
diff --git a/lib/manager/npm/package.js b/lib/manager/npm/package.js
index c042fbde6b..17511efc06 100644
--- a/lib/manager/npm/package.js
+++ b/lib/manager/npm/package.js
@@ -45,18 +45,25 @@ async function getPackageUpdates(config) {
);
}
} else {
- // If dependency lookup fails then warn and return
- const result = {
- type: 'warning',
- message: 'Failed to look up dependency',
- };
if (
config.updateLockFiles &&
(config.yarnLock || config.packageLock || config.npmShrinkwrap)
) {
- result.message +=
- '. This will block *all* dependencies from being updated due to presence of lock file.';
+ // Config error
+ const error = new Error('config-validation');
+ error.configFile = config.packageFile;
+ error.validationError = `Failed to look up npm dependency \`${
+ config.depName
+ }\``;
+ error.validationMessage =
+ 'This dependency lookup failure will cause all lock file updates to fail. Please either remove the dependency, or remove the lock file, or add npm authentication, or set `updateLockFiles` to false in your config.';
+ throw error;
}
+ // If dependency lookup fails then warn and return
+ const result = {
+ type: 'warning',
+ message: `Failed to look up dependency ${config.depName}`,
+ };
results = [result];
logger.info({ dependency: config.depName }, result.message);
}
diff --git a/lib/workers/repository/error-config.js b/lib/workers/repository/error-config.js
index c203df361f..a8dcb86946 100644
--- a/lib/workers/repository/error-config.js
+++ b/lib/workers/repository/error-config.js
@@ -4,9 +4,9 @@ module.exports = {
async function raiseConfigWarningIssue(config, error) {
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 renovations until it is fixed.\n\n`;
+ 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`;
if (error.configFile) {
- body += `Configuration file: \`${error.configFile}\`\n`;
+ body += `File: \`${error.configFile}\`\n`;
}
body += `Error type: ${error.validationError}\n`;
if (error.validationMessage) {
@@ -23,7 +23,7 @@ async function raiseConfigWarningIssue(config, error) {
'Action Required: Fix Renovate Configuration',
body
);
- if (res) {
+ if (res === 'created') {
logger.warn({ configError: error, res }, 'Config Warning');
}
}
diff --git a/lib/workers/repository/updates/index.js b/lib/workers/repository/updates/index.js
index 856f60846a..cdfcc1baaf 100644
--- a/lib/workers/repository/updates/index.js
+++ b/lib/workers/repository/updates/index.js
@@ -10,6 +10,7 @@ async function determineUpdates(input) {
logger.debug('determineUpdates()');
logger.trace({ config });
config = await determineRepoUpgrades(config);
+ platform.ensureIssueClosing('Action Required: Fix Renovate Configuration');
config = branchifyUpgrades(config);
logger.debug('Finished determining upgrades');
return config;
diff --git a/test/manager/npm/__snapshots__/package.spec.js.snap b/test/manager/npm/__snapshots__/package.spec.js.snap
index de45e88d21..b3802a82da 100644
--- a/test/manager/npm/__snapshots__/package.spec.js.snap
+++ b/test/manager/npm/__snapshots__/package.spec.js.snap
@@ -6,30 +6,10 @@ Array [
]
`;
-exports[`lib/workers/package/npm getPackageUpdates returns error if no npm scoped dep found 1`] = `
-Array [
- Object {
- "message": "Failed to look up dependency. This will block *all* dependencies from being updated due to presence of lock file.",
- "repositoryUrl": null,
- "type": "warning",
- },
-]
-`;
-
exports[`lib/workers/package/npm getPackageUpdates returns warning if no npm dep found 1`] = `
Array [
Object {
- "message": "Failed to look up dependency",
- "repositoryUrl": null,
- "type": "warning",
- },
-]
-`;
-
-exports[`lib/workers/package/npm getPackageUpdates returns warning if no npm dep found and lock file 1`] = `
-Array [
- Object {
- "message": "Failed to look up dependency. This will block *all* dependencies from being updated due to presence of lock file.",
+ "message": "Failed to look up dependency some-dep",
"repositoryUrl": null,
"type": "warning",
},
diff --git a/test/manager/npm/package.spec.js b/test/manager/npm/package.spec.js
index 9e7f2f3f39..4227ad3fe3 100644
--- a/test/manager/npm/package.spec.js
+++ b/test/manager/npm/package.spec.js
@@ -48,21 +48,26 @@ describe('lib/workers/package/npm', () => {
expect(res[0].type).toEqual('warning');
expect(npmApi.getDependency.mock.calls.length).toBe(1);
});
- it('returns warning if no npm dep found and lock file', async () => {
+ it('throws error if no npm dep found and lock file', async () => {
config.packageLock = 'some package lock';
- const res = await npm.getPackageUpdates(config);
- expect(res).toMatchSnapshot();
- expect(res).toHaveLength(1);
- expect(res[0].type).toEqual('warning');
- expect(npmApi.getDependency.mock.calls.length).toBe(1);
+ let e;
+ try {
+ await npm.getPackageUpdates(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
});
it('returns error if no npm scoped dep found', async () => {
config.depName = '@foo/something';
config.yarnLock = '# some yarn lock';
- const res = await npm.getPackageUpdates(config);
- expect(res).toMatchSnapshot();
- expect(res).toHaveLength(1);
- expect(res[0].type).toEqual('warning');
+ let e;
+ try {
+ await npm.getPackageUpdates(config);
+ } catch (err) {
+ e = err;
+ }
+ expect(e).toBeDefined();
});
it('returns warning if warning found', async () => {
npmApi.getDependency.mockReturnValueOnce({});
--
GitLab