Skip to content

Commit

Permalink
fix(composer): invoke callback only once
Browse files Browse the repository at this point in the history
In Node.js Streams, if an error is omitted downstream, that error can be
propagated back through connected streams. In gulp-uglify, this error
was being thrown by the next callback, being caught, and callback called
a second time.

This CL ensures the callback is called only once, by calling it outside
the try..catch block wrapping the minifier.

[terinjokes@gmail.com: amended change, rewrote tests and commit message]
Signed-off-by: Terin Stock <terinjokes@gmail.com>
  • Loading branch information
robpalme authored and terinjokes committed Jul 11, 2018
1 parent 2c7f656 commit 27b26d4
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 4 deletions.
10 changes: 6 additions & 4 deletions composer.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,14 @@ module.exports = function(uglify, logger) {
return function(opts) {
var minifier = minify(uglify, logger)(opts);
return through.obj(function(file, encoding, callback) {
var newFile = null;
var err = null;
try {
var newFile = minifier(file);
callback(null, newFile);
} catch (err) {
callback(err);
newFile = minifier(file);
} catch (e) {
err = e;
}
callback(err, newFile);
});
};
};
73 changes: 73 additions & 0 deletions test/composer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
'use strict';
var test = require('tape-catch');
var assert = require('assert');
var Buffer = require('safe-buffer').Buffer;
var Vinyl = require('vinyl');
var td = require('testdouble');
var through = require('through2');
var composer = require('../composer');
var GulpUglifyError = require('../lib/gulp-uglify-error');

test('composer should forward errors', function(t) {
var badJsFile = new Vinyl({
cwd: '/',
base: '/test/',
path: '/test/file.js',
contents: Buffer.from('invalid js')
});

var uglify = td.object(['minify']);
var logger = td.object(['warn']);
var composed = composer(uglify, logger)({});

assert.throws(function() {
composed.write(badJsFile);
}, GulpUglifyError);

td.reset();
t.end();
});

test("compose doesn't invoke callback twice", function(t) {
var expectedErr = new Error();
var jsFile = new Vinyl({
cwd: '/',
base: '/test/',
path: '/test/file.js',
contents: Buffer.from('var x = 123')
});

var thrower = through.obj(function() {
throw expectedErr;
});

var uglify = td.object(['minify']);
var logger = td.object(['warn']);
td.when(
uglify.minify(
{'file.js': 'var x = 123'},
{
output: {}
}
)
).thenReturn({
code: '',
map: {}
});

var composed = composer(uglify, logger)({});
composed.pipe(thrower);

assert.throws(
function() {
composed.write(jsFile);
},
function(err) {
assert.strictEqual(err, expectedErr);
return true;
}
);

td.reset();
t.end();
});

0 comments on commit 27b26d4

Please sign in to comment.