feat: stop branch processing after lock file error or pin dependencies (#768)

If a repository has a lock file error (e.g. can’t look up a private module) then it will no longer attempt to create every branch. Instead, it will error/exit after the first branch. Additionally, “Pin Dependencies” has been sorted to be first and further branches won’t be added or updated until Pin Dependencies has been merged.

lib/workers/branch/index.js

@@ -52,7 +52,7 @@ async function processBranch(branchConfig) {
     }
     Object.assign(config, await getUpdatedLockFiles(config));
     if (config.lockFileError) {
-      throw new Error('lockFileError');
+      return 'lockFileError';
     }
     if (config.updatedLockFiles.length) {
       logger.debug(
@@ -81,11 +81,7 @@ async function processBranch(branchConfig) {
       return 'automerged';
     }
   } catch (err) {
-    if (err.message !== 'lockFileError') {
-      logger.error({ err }, `Error updating branch: ${err.message}`);
-    } else {
-      logger.info('Error updating branch');
-    }
+    logger.error({ err }, `Error updating branch: ${err.message}`);
     // Don't throw here - we don't want to stop the other renovations
     return 'error';
   }

lib/workers/repository/index.js

@@ -10,9 +10,20 @@ const cleanup = require('./cleanup');
 const { decryptConfig } = require('../../config/decrypt');
 
 module.exports = {
+  pinDependenciesFirst,
   renovateRepository,
 };
 
+function pinDependenciesFirst(a, b) {
+  if (a.type === 'pin') {
+    return false;
+  }
+  if (b.type === 'pin') {
+    return true;
+  }
+  return a.branchName > b.branchName;
+}
+
 async function renovateRepository(repoConfig, token) {
   let config = { ...repoConfig };
   const { logger } = config;
@@ -111,22 +122,28 @@ async function renovateRepository(repoConfig, token) {
       logger.debug(`Updating ${branchUpgrades.length} branch(es)`);
       logger.trace({ config: branchUpgrades }, 'branchUpgrades');
       if (config.repoIsOnboarded) {
+        logger.info(`Processing ${branchUpgrades.length} branch(es)`);
+        // eslint-disable-next-line no-loop-function
+        branchUpgrades.sort(pinDependenciesFirst);
         for (const branchUpgrade of branchUpgrades) {
-          if (!baseBranchUpdated) {
-            const branchResult = await branchWorker.processBranch(
-              branchUpgrade,
-              config.errors,
-              config.warnings
-            );
-            if (branchResult === 'automerged') {
-              // Stop procesing other branches because base branch has been changed by an automerge
-              logger.info('Restarting repo renovation after automerge');
-              baseBranchUpdated = true;
-            }
-          } else {
-            logger.debug(
-              `Skipping branchUpgrade as base branch has been modified`
+          const branchResult = await branchWorker.processBranch(
+            branchUpgrade,
+            config.errors,
+            config.warnings
+          );
+          if (branchResult === 'automerged') {
+            // Stop procesing other branches because base branch has been changed by an automerge
+            logger.info('Restarting repo renovation after automerge');
+            baseBranchUpdated = true;
+            break;
+          } else if (branchResult === 'lockFileError') {
+            logger.info('Lock file error - stopping branch updates');
+            break;
+          } else if (branchUpgrade.type === 'pin') {
+            logger.info(
+              'Stopping branch processing until Pin Dependencies is merged'
             );
+            break;
           }
         }
         branchList = branchUpgrades.map(upgrade => upgrade.branchName);

test/workers/repository/__snapshots__/index.spec.js.snap

+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`workers/repository pinDependenciesFirst returns pin first 1`] = `
+Array [
+  Object {
+    "branchName": "d",
+    "type": "pin",
+  },
+  Object {
+    "branchName": "a",
+  },
+  Object {
+    "branchName": "b",
+  },
+  Object {
+    "branchName": "c",
+  },
+]
+`;
+
+exports[`workers/repository pinDependenciesFirst returns pin first 2`] = `
+Array [
+  Object {
+    "branchName": "d",
+    "type": "pin",
+  },
+  Object {
+    "branchName": "a",
+  },
+  Object {
+    "branchName": "b",
+  },
+  Object {
+    "branchName": "c",
+  },
+]
+`;
+
+exports[`workers/repository pinDependenciesFirst returns sorted if no pin 1`] = `
+Array [
+  Object {
+    "branchName": "a",
+  },
+  Object {
+    "branchName": "b",
+  },
+  Object {
+    "branchName": "c",
+  },
+]
+`;

test/workers/repository/index.spec.js

@@ -8,6 +8,37 @@ const upgrades = require('../../../lib/workers/repository/upgrades');
 const logger = require('../../_fixtures/logger');
 
 describe('workers/repository', () => {
+  describe('pinDependenciesFirst', () => {
+    it('returns sorted if no pin', () => {
+      const arr = [
+        { branchName: 'a' },
+        { branchName: 'c' },
+        { branchName: 'b' },
+      ];
+      arr.sort(repositoryWorker.pinDependenciesFirst);
+      expect(arr).toMatchSnapshot();
+    });
+    it('returns pin first', () => {
+      const arr = [
+        { branchName: 'a' },
+        { branchName: 'c' },
+        { branchName: 'd', type: 'pin' },
+        { branchName: 'b' },
+      ];
+      arr.sort(repositoryWorker.pinDependenciesFirst);
+      expect(arr).toMatchSnapshot();
+    });
+    it('returns pin first', () => {
+      const arr = [
+        { branchName: 'd', type: 'pin' },
+        { branchName: 'a' },
+        { branchName: 'c' },
+        { branchName: 'b' },
+      ];
+      arr.sort(repositoryWorker.pinDependenciesFirst);
+      expect(arr).toMatchSnapshot();
+    });
+  });
   describe('renovateRepository', () => {
     let config;
     beforeEach(() => {
@@ -175,6 +206,32 @@ describe('workers/repository', () => {
       expect(branchWorker.processBranch.mock.calls).toHaveLength(3);
       expect(config.logger.error.mock.calls).toHaveLength(0);
     });
+    it('stops branchWorker after lockFileError', async () => {
+      config.packageFiles = ['package.json'];
+      config.hasRenovateJson = true;
+      onboarding.getOnboardingStatus.mockReturnValue(true);
+      upgrades.branchifyUpgrades.mockReturnValueOnce({
+        upgrades: [{}, {}, {}],
+      });
+      branchWorker.processBranch.mockReturnValue('lockFileError');
+      await repositoryWorker.renovateRepository(config);
+      expect(upgrades.branchifyUpgrades.mock.calls).toHaveLength(1);
+      expect(branchWorker.processBranch.mock.calls).toHaveLength(1);
+      expect(config.logger.error.mock.calls).toHaveLength(0);
+    });
+    it('stops branchWorker after pin', async () => {
+      config.packageFiles = ['package.json'];
+      config.hasRenovateJson = true;
+      onboarding.getOnboardingStatus.mockReturnValue(true);
+      upgrades.branchifyUpgrades.mockReturnValueOnce({
+        upgrades: [{ type: 'pin' }, {}, {}],
+      });
+      branchWorker.processBranch.mockReturnValue('done');
+      await repositoryWorker.renovateRepository(config);
+      expect(upgrades.branchifyUpgrades.mock.calls).toHaveLength(1);
+      expect(branchWorker.processBranch.mock.calls).toHaveLength(1);
+      expect(config.logger.error.mock.calls).toHaveLength(0);
+    });
     it('swallows errors', async () => {
       apis.initApis.mockImplementationOnce(() => {
         throw new Error('bad init');