Skip to content

Commit

Permalink
fix(errors): emit errors of type GulpUglifyError
Browse files Browse the repository at this point in the history
Use a lightweight wrapper to construct custom Error subtypes to replace
`PluginError`. Original errors from UglifyJS are now preserved, and
available as the "cause" property.

BREAKING CHANGE: Emitted errors are of a new type. Original UglifyJS
error message are preserved instead of the attempt to normalize them.
The constructor is exported as "GulpUglifyError" to allow for instance
checks.
  • Loading branch information
terinjokes committed Aug 1, 2016
1 parent 5632cee commit 1232c3c
Show file tree
Hide file tree
Showing 9 changed files with 66 additions and 45 deletions.
14 changes: 10 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -89,11 +89,17 @@ UglifyJS's behavior.
## Errors

`gulp-uglify` emits an 'error' event if it is unable to minify a specific file.
Wherever possible, the PluginError object will contain the following properties:
The GulpUglifyError constructor is exported by this plugin for `instanceof` checks.
It contains the following properties:

- `fileName`
- `lineNumber`
- `message`
- `fileName`: The full file path for the file being minified.
- `cause`: The original UglifyJS error, if avialable.

Most UglifyJS error messages have the following properties:

- `message` (or `msg`)
- `filename`
- `line`

## Using a Different UglifyJS

Expand Down
3 changes: 3 additions & 0 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
'use strict';
var uglify = require('uglify-js');
var minifier = require('./minifier');
var GulpUglifyError = require('./lib/gulp-uglify-error');

module.exports = function (opts) {
return minifier(opts, uglify);
};

module.exports.GulpUglifyError = GulpUglifyError;
33 changes: 12 additions & 21 deletions lib/create-error.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,13 @@
'use strict';
var PluginError = require('gulp-util/lib/PluginError');

var pluginName = 'gulp-uglify';

module.exports = function createError(file, err) {
if (typeof err === 'string') {
return new PluginError(pluginName, file.path + ': ' + err, {
fileName: file.path,
showStack: false
});
}

var msg = err.message || err.msg || 'unspecified error';

return new PluginError(pluginName, file.path + ': ' + msg, {
fileName: file.path,
lineNumber: err.line,
stack: err.stack,
showStack: false
});
};
var curry = require('lodash/fp/curry');
var GulpUglifyError = require('./gulp-uglify-error');

function createError(file, msg, cause) {
var perr = new GulpUglifyError(msg, cause);
perr.plugin = 'gulp-uglify';
perr.fileName = file.path;
perr.showStack = false;
return perr;
}

module.exports = curry(createError);
4 changes: 4 additions & 0 deletions lib/gulp-uglify-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
'use strict';
var makeErrorCause = require('make-error-cause');

module.exports = makeErrorCause('GulpUglifyError');
8 changes: 4 additions & 4 deletions minifier.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
'use strict';
var through = require('through2');
var PluginError = require('gulp-util/lib/PluginError');
var applySourceMap = require('vinyl-sourcemaps-apply');
var saveLicense = require('uglify-save-license');
var isObject = require('lodash/fp/isObject');
Expand All @@ -11,6 +10,7 @@ var _ = require('lodash/fp/placeholder');
var defaultsDeep = require('lodash/fp/defaultsDeep');
var log = require('./lib/log');
var createError = require('./lib/create-error');
var GulpUglifyError = require('./lib/gulp-uglify-error');

var reSourceMapComment = /\n\/\/# sourceMappingURL=.+?$/;

Expand Down Expand Up @@ -59,7 +59,7 @@ module.exports = function (opts, uglify) {
}

if (file.isStream()) {
return callback(createError(file, 'Streaming not supported'));
return callback(createError(file, 'Streaming not supported', null));
}

if (file.sourceMap) {
Expand All @@ -79,9 +79,9 @@ module.exports = function (opts, uglify) {
var m = uglify.minify(map, options);
m.code = new Buffer(m.code.replace(reSourceMapComment, ''));
return m;
}, createError.bind(null, file));
}, createError(file, 'unable to minify JavaScript'));

if (mangled instanceof PluginError) {
if (mangled instanceof GulpUglifyError) {
return callback(mangled);
}

Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@
"author": "Terin Stock <[email protected]>",
"bugs": "https://github.com/terinjokes/gulp-uglify/issues",
"dependencies": {
"gulp-util": "^3.0.0",
"gulplog": "^1.0.0",
"has-gulplog": "^0.1.0",
"lodash": "^4.13.1",
"make-error-cause": "^1.1.1",
"through2": "^2.0.0",
"uglify-js": "2.7.0",
"uglify-save-license": "^0.4.1",
Expand Down
25 changes: 20 additions & 5 deletions test/create-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
var test = require('tape');
var Vinyl = require('vinyl');
var createError = require('../lib/create-error');
var GulpUglifyError = require('../lib/gulp-uglify-error');

var testOkContentsInput = '"use strict"; (function(console, first, second) { console.log(first + second) }(5, 10))';
var testFile = new Vinyl({
Expand All @@ -11,11 +12,25 @@ var testFile = new Vinyl({
contents: new Buffer(testOkContentsInput)
});

test('should report "unspecified error" if err has no msg or message properties', function (t) {
t.plan(3);
var e = createError(testFile, {});
test('should have expected error message', function (t) {
t.plan(5);
var e = createError(testFile, 'error message text', null);

t.ok(e instanceof Error, 'argument should be of type error');
t.ok(e instanceof Error, 'argument should be of type Error');
t.ok(e instanceof GulpUglifyError, 'argument should be of type GulpUglifyError');
t.equal(e.plugin, 'gulp-uglify', 'error is from gulp-uglify');
t.true(e.message.match(/unspecified error$/), 'error should not be specified');
t.equal(e.message, 'error message text');
t.notOk(e.cause, 'should not contain a cause');
});

test('should wrap cause', function (t) {
t.plan(5);
var cause = new Error('boom!');
var e = createError(testFile, 'error message text', cause);

t.ok(e instanceof Error, 'argument should be of type Error');
t.ok(e instanceof GulpUglifyError, 'argument should be of type GulpUglifyError');
t.equal(e.plugin, 'gulp-uglify', 'error is from gulp-uglify');
t.ok(e.message.match(/^error message text/));
t.equal(e.cause, cause);
});
15 changes: 9 additions & 6 deletions test/err.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';
var test = require('tape');
var Vinyl = require('vinyl');
var GulpUglifyError = require('../lib/gulp-uglify-error');
var gulpUglify = require('../');

var testContentsInput = 'function errorFunction(error)\n{';
Expand All @@ -21,7 +22,7 @@ var testFile2 = new Vinyl({
});

test('should report files in error', function (t) {
t.plan(6);
t.plan(7);

var stream = gulpUglify();

Expand All @@ -30,10 +31,11 @@ test('should report files in error', function (t) {
});

stream.on('error', function (e) {
t.ok(e instanceof Error, 'argument should be of type error');
t.ok(e instanceof Error, 'argument should be of type Error');
t.ok(e instanceof GulpUglifyError, 'argument should be of type GulpUglifyError');
t.equal(e.plugin, 'gulp-uglify', 'error is from gulp-uglify');
t.equal(e.fileName, testFile1.path, 'error reports correct file name');
t.equal(e.lineNumber, 2, 'error reports correct line number');
t.equal(e.cause.line, 2, 'error reports correct line number');
t.ok(e.stack, 'error has a stack');
t.false(e.showStack, 'error is configured to not print the stack');
});
Expand All @@ -43,7 +45,7 @@ test('should report files in error', function (t) {
});

test('shouldn\'t blow up when given output options', function (t) {
t.plan(5);
t.plan(6);

var stream = gulpUglify({
output: {
Expand All @@ -56,8 +58,9 @@ test('shouldn\'t blow up when given output options', function (t) {
});

stream.on('error', function (e) {
t.ok(e instanceof Error, 'argument should be of type error');
t.equals(e.message, testFile2.path + ': `exportAll` is not a supported option');
t.ok(e instanceof Error, 'argument should be of type Error');
t.ok(e instanceof GulpUglifyError, 'argument should be of type GulpUglifyError');
t.equals(e.cause.msg, '`exportAll` is not a supported option');
t.equal(e.plugin, 'gulp-uglify', 'error is from gulp-uglify');
t.equal(e.fileName, testFile2.path, 'error reports correct file name');
t.false(e.showStack, 'error is configured to not print the stack');
Expand Down
7 changes: 3 additions & 4 deletions test/streams.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
var Readable = require('stream').Readable;
var test = require('tape');
var Vinyl = require('vinyl');
var PluginError = require('gulp-util/lib/PluginError');
var GulpUglifyError = require('../lib/gulp-uglify-error');
var gulpUglify = require('../');

var testFile1 = new Vinyl({
Expand All @@ -21,12 +21,11 @@ test('should emit error for stream files', function (t) {
t.fail('should emit error for streams');
}).on('error', function (e) {
t.pass('emitted error');
t.ok(e instanceof PluginError, 'error is a PluginError');
t.ok(e instanceof GulpUglifyError, 'error is a GulpUglifyError');
t.equal(e.plugin, 'gulp-uglify', 'error is from gulp-uglify');
t.equal(e.fileName, testFile1.path, 'error reports the correct file');

// t.ok(e.stack, 'error has a stack');
t.skip('error should have a stack');
t.ok(e.stack, 'error has a stack');
t.false(e.showStack, 'error is configured to not print stack');
});

Expand Down

0 comments on commit 1232c3c

Please sign in to comment.