Skip to content

Commit

Permalink
fix(launcher): handle ENOENT error, do not retry
Browse files Browse the repository at this point in the history
If the binary does not exists:
- handle the error (Node 0.10+ throws unhandled error events)
- show nicer message
- do not retry

Closes #452
  • Loading branch information
vojtajina committed May 11, 2013
1 parent 0d3d97e commit 7d790b2
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 4 deletions.
30 changes: 26 additions & 4 deletions lib/launchers/Base.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ var BaseBrowser = function(id, emitter, captureTimeout, retryLimit) {

this.start = function(url) {
capturingUrl = url;
self.state = BEING_CAPTURED;

try {
log.debug('Creating temp dir at ' + self._tempDir);
fs.mkdirSync(self._tempDir);
} catch (e) {}

self._start(capturingUrl + '?id=' + self.id);
self.state = BEING_CAPTURED;

if (captureTimeout) {
setTimeout(self._onTimeout, captureTimeout);
Expand Down Expand Up @@ -104,13 +104,35 @@ var BaseBrowser = function(id, emitter, captureTimeout, retryLimit) {
self._process = spawn(cmd, args);

var errorOutput = '';
self._process.stderr.on('data', function(data) {
errorOutput += data.toString();
});

self._process.on('close', function(code) {
self._onProcessExit(code, errorOutput);
});

self._process.on('error', function(err) {
if (err.code === 'ENOENT') {
retryLimit = 0;
errorOutput = 'Can not find the binary ' + cmd + '\n\t' +
'Please set env variable ' + self.ENV_CMD;
} else {
errorOutput += err.toString();
}
});

// Node 0.8 does not emit the error
if (process.versions.node.indexOf('0.8') === 0) {
self._process.stderr.on('data', function(data) {
var msg = data.toString();

if (msg.indexOf('No such file or directory') !== -1) {
retryLimit = 0;
errorOutput = 'Can not find the binary ' + cmd + '\n\t' +
'Please set env variable ' + self.ENV_CMD;
} else {
errorOutput += msg;
}
});
}
};


Expand Down
25 changes: 25 additions & 0 deletions test/unit/launchers/Base.spec.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ describe 'launchers Base', ->
globals =
process:
platform: 'darwin'
versions: node: '0.10.x'
env:
TMP: '/tmp'
nextTick: process.nextTick
Expand Down Expand Up @@ -74,6 +75,30 @@ describe 'launchers Base', ->
expect(browser._start).to.have.been.calledWith '/capture/url?id=123'


it 'should handle spawn ENOENT error and not even retry', (done) ->
browser = new m.BaseBrowser 123, new events.EventEmitter, 0, 3
browser.DEFAULT_CMD = darwin: '/usr/bin/browser'

error = new Error 'spawn ENOENT'
error.code = 'ENOENT'

browser.start '/capture/url'

spawnProcess = mockSpawn._processes[0]
mockSpawn.reset()

spawnProcess.emit 'error', error
spawnProcess.emit 'close', 1

# do not retry
expect(mockSpawn).not.to.have.been.called

browser.kill done

# do not kill already dead process
expect(spawnProcess.kill).to.not.have.been.called


describe 'kill', ->
it 'should just fire done if already killed', (done) ->
browser = new m.BaseBrowser 123, new events.EventEmitter, 0, 1 # disable retry
Expand Down

0 comments on commit 7d790b2

Please sign in to comment.