From e5baeee98f8f1187098eaacd38abcaf722f99022 Mon Sep 17 00:00:00 2001 From: acheronfail <acheronfail@gmail.com> Date: Thu, 19 Mar 2020 07:20:26 +0000 Subject: [PATCH] fix: expose error when not using docker and exec fails (#5688) --- .../__snapshots__/artifacts.spec.ts.snap | 6 --- lib/util/exec/docker/index.ts | 2 +- lib/util/exec/exec.spec.ts | 54 ++++++++++++++++++- lib/util/exec/index.ts | 8 ++- 4 files changed, 60 insertions(+), 10 deletions(-) diff --git a/lib/manager/cocoapods/__snapshots__/artifacts.spec.ts.snap b/lib/manager/cocoapods/__snapshots__/artifacts.spec.ts.snap index eb6874fe87..a257cf7fb8 100644 --- a/lib/manager/cocoapods/__snapshots__/artifacts.spec.ts.snap +++ b/lib/manager/cocoapods/__snapshots__/artifacts.spec.ts.snap @@ -153,12 +153,6 @@ Array [ "timeout": 900000, }, }, - Object { - "cmd": "docker ps --filter name=renovate_cocoapods -aq | xargs --no-run-if-empty docker rm -f", - "options": Object { - "encoding": "utf-8", - }, - }, ] `; diff --git a/lib/util/exec/docker/index.ts b/lib/util/exec/docker/index.ts index 7f1b735f85..e3ff78f93f 100644 --- a/lib/util/exec/docker/index.ts +++ b/lib/util/exec/docker/index.ts @@ -128,7 +128,7 @@ export async function removeDockerContainer(image): Promise<void> { } else { logger.trace({ image, containerName }, 'No running containers to remove'); } - } catch (err) { + } catch (err) /* istanbul ignore next */ { logger.trace({ err }, 'removeDockerContainer err'); logger.info({ image, containerName }, 'Could not remove Docker container'); } diff --git a/lib/util/exec/exec.spec.ts b/lib/util/exec/exec.spec.ts index 88a71666d9..8226157cee 100644 --- a/lib/util/exec/exec.spec.ts +++ b/lib/util/exec/exec.spec.ts @@ -10,7 +10,7 @@ import { VolumeOption, } from './common'; import { envMock } from '../../../test/execUtil'; -import { resetPrefetchedImages } from './docker'; +import * as dockerModule from './docker'; const cpExec: jest.Mock<typeof _cpExec> = _cpExec as any; @@ -42,8 +42,9 @@ describe(`Child process execution wrapper`, () => { }; beforeEach(() => { - resetPrefetchedImages(); + dockerModule.resetPrefetchedImages(); jest.resetAllMocks(); + jest.restoreAllMocks(); jest.resetModules(); processEnvOrig = process.env; trustLevelOrig = global.trustLevel; @@ -450,4 +451,53 @@ describe(`Child process execution wrapper`, () => { expect(actualCmd).toMatchSnapshot(); }); + + it('only calls removeDockerContainer in catch block is useDocker is set', async () => { + cpExec.mockImplementation(() => { + throw new Error('some error occurred'); + }); + + const removeDockerContainerSpy = jest.spyOn( + dockerModule, + 'removeDockerContainer' + ); + + const promise = exec('foobar', {}); + await expect(promise).rejects.toThrow('some error occurred'); + expect(removeDockerContainerSpy).toHaveBeenCalledTimes(0); + }); + + it('wraps error if removeDockerContainer throws an error', async () => { + setExecConfig({ binarySource: BinarySource.Docker }); + cpExec.mockImplementation(() => { + throw new Error('some error occurred'); + }); + jest + .spyOn(dockerModule, 'generateDockerCommand') + .mockImplementation((): any => 'asdf'); + + // The `removeDockerContainer` function is called once before it's used in the `catch` block. + // We want it to fail in the catch block so we can assert the error is wrapped. + let calledOnce = false; + const removeDockerContainerSpy = jest.spyOn( + dockerModule, + 'removeDockerContainer' + ); + removeDockerContainerSpy.mockImplementation((): any => { + if (!calledOnce) { + calledOnce = true; + return Promise.resolve(); + } + + return Promise.reject(new Error('removeDockerContainer failed')); + }); + + const promise = exec('foobar', { docker }); + await expect(promise).rejects.toThrow( + new Error( + 'Error: "removeDockerContainer failed" - Original Error: "some error occurred"' + ) + ); + expect(removeDockerContainerSpy).toHaveBeenCalledTimes(2); + }); }); diff --git a/lib/util/exec/index.ts b/lib/util/exec/index.ts index 38872f617a..fd9f139119 100644 --- a/lib/util/exec/index.ts +++ b/lib/util/exec/index.ts @@ -158,7 +158,13 @@ export async function exec( } catch (err) { logger.trace({ err }, 'rawExec err'); clearTimeout(timer); - await removeDockerContainer(docker.image); + if (useDocker) { + await removeDockerContainer(docker.image).catch(removeErr => { + throw new Error( + `Error: "${removeErr.message}" - Original Error: "${err.message}"` + ); + }); + } throw err; } clearTimeout(timer); -- GitLab