From e3da458b376871c3ce72d6984d14bf1ee668b04b Mon Sep 17 00:00:00 2001 From: Tim Alby Date: Fri, 10 Apr 2020 12:28:11 +0200 Subject: [PATCH] retry once on EPIPE errors Co-Authored-By: Jakob Ackermann --- app/js/DockerRunner.js | 34 +++++++++++------- test/unit/js/DockerRunnerTests.js | 58 +++++++++++++++++++++++++++++++ 2 files changed, 80 insertions(+), 12 deletions(-) diff --git a/app/js/DockerRunner.js b/app/js/DockerRunner.js index 3594b3a..32bcf70 100644 --- a/app/js/DockerRunner.js +++ b/app/js/DockerRunner.js @@ -407,19 +407,29 @@ module.exports = DockerRunner = { }) } ) - return container.inspect(function(error, stats) { - if ((error != null ? error.statusCode : undefined) === 404) { - return createAndStartContainer() - } else if (error != null) { - logger.err( - { container_name: name, error }, - 'unable to inspect container to start' - ) - return callback(error) - } else { - return startExistingContainer() + + const callbackWithRetry = error => { + if (error.message.match(/EPIPE/)) { + return inspectContainer(container, callback) } - }) + callback(error) + } + + var inspectContainer = (container, innerCallback) => + container.inspect(function(error, stats) { + if ((error != null ? error.statusCode : undefined) === 404) { + return createAndStartContainer() + } else if (error != null) { + logger.err( + { container_name: name, error }, + 'unable to inspect container to start' + ) + return innerCallback(error) + } else { + return startExistingContainer() + } + }) + inspectContainer(container, callbackWithRetry) }, attachToContainer(containerId, attachStreamHandler, attachStartCallback) { diff --git a/test/unit/js/DockerRunnerTests.js b/test/unit/js/DockerRunnerTests.js index 3daded8..7284c6e 100644 --- a/test/unit/js/DockerRunnerTests.js +++ b/test/unit/js/DockerRunnerTests.js @@ -36,6 +36,7 @@ describe('DockerRunner', function() { 'logger-sharelatex': (this.logger = { log: sinon.stub(), error: sinon.stub(), + err: sinon.stub(), info: sinon.stub(), warn: sinon.stub() }), @@ -387,6 +388,63 @@ describe('DockerRunner', function() { }) }) + describe('when inspect always fails with EPIPE error', function() { + beforeEach(function() { + this.error = new Error('write EPIPE') + this.container.inspect = sinon.stub().yields(this.error) + this.container.start = sinon.stub().yields() + + this.DockerRunner.startContainer( + this.options, + this.volumes, + () => {}, + this.callback + ) + }) + + it('should retry once', function() { + sinon.assert.callOrder( + this.container.inspect, + this.container.inspect, + this.callback + ) + }) + + it('should call back with error', function() { + sinon.assert.calledWith(this.callback, this.error) + }) + }) + + describe('when inspect fails once with EPIPE error', function() { + beforeEach(function() { + this.container.inspect = sinon.stub() + this.container.inspect.onFirstCall().yields(new Error('write EPIPE')) + this.container.inspect.onSecondCall().yields() + this.container.start = sinon.stub().yields() + + this.DockerRunner.startContainer( + this.options, + this.volumes, + () => {}, + this.callback + ) + }) + + it('should retry once and start container', function() { + sinon.assert.callOrder( + this.container.inspect, + this.container.inspect, + this.DockerRunner.attachToContainer, + this.container.start, + this.callback + ) + }) + + it('should call back without error', function() { + sinon.assert.calledWith(this.callback, null) + }) + }) + describe('when the container does not exist', function() { beforeEach(function() { const exists = false