From 1a380f5532a95236eb066d44398d9a9c7b34b37f Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 21 Feb 2017 23:25:25 +0200 Subject: [PATCH 1/9] Improve minifiers to capture error from UglifyJS. The error messages which come from UglifyJS tend to be quite cryptic, as seen in issues like meteor/meteor#8370 or meteor/meteor#8020. The file, line, and column are provided, however the message is garbled and the stacktrace long and acutely harrowing. Since these errors are occurring on automatically concatenated files, even the line number is sometimes not helpful. Additionally, sourceMaps are not available in production builds, intentionally. (I wasn't able to access them from `file.getSourceMap()` or `file.sourceMap` at all.) In addition to actually providing the name of the encapsulating file, which provides _some_visibility, this commit implements a parser around the UglifyJS error which detects the error and location information of the error, seeks to the line in the concatenated source, reads the inline filename, and provides it in the output. Crude, but incredibly helpful in diagnosing this problem until a better solution is reached. --- .../standard-minifier-js/plugin/minify-js.js | 101 +++++++++++++++++- 1 file changed, 98 insertions(+), 3 deletions(-) diff --git a/packages/standard-minifier-js/plugin/minify-js.js b/packages/standard-minifier-js/plugin/minify-js.js index b9a5248826..37a4038b4d 100644 --- a/packages/standard-minifier-js/plugin/minify-js.js +++ b/packages/standard-minifier-js/plugin/minify-js.js @@ -32,13 +32,110 @@ UglifyJSMinifier.prototype.processFilesForBundle = function (files, options) { } }; + function maybeThrowMinifyErrorBySourceFile(error, file) { + var minifierErrorRegex = /\(line: (\d+), col: (\d+), pos: (\d+)\)/; + var parseError = minifierErrorRegex.exec(error.toString()); + + if (parseError) { + var lineErrorMessage = parseError[0]; + var lineErrorLineNumber = parseError[1]; + + var parseErrorContentIndex = lineErrorLineNumber - 1; + + // Unlikely, since we have a multi-line fixed header in this file. + if (parseErrorContentIndex < 0) { + return; + } + + /* + + What we're parsing looks like this: + + ///////////////////////////////////////// + // // + // path/to/file.js // + // // + ///////////////////////////////////////// + // 1 + var illegalECMAScript = true; // 2 + // 3 + ///////////////////////////////////////// + + Btw, the above code is intentionally not newer ECMAScript so + we don't break ourselves. + + */ + + var contents = file.getContentsAsString().split(/\n/); + var lineContent = contents[parseErrorContentIndex]; + + // Try to grab the line number, which sometimes doesn't exist on + // line, abnormally-long lines in a larger block. + var lineSrcLineParts = /^(.*?)(?:\s*\/\/ (\d+))?$/.exec(lineContent); + + // The line didn't match at all? Let's just not try. + if (!lineSrcLineParts) { + return; + } + + var lineSrcLineContent = lineSrcLineParts[1]; + var lineSrcLineNumber = lineSrcLineParts[2]; + + // Count backward from the failed line to find the filename. + for (var c = parseErrorContentIndex - 1; c >= 0; c--) { + var sourceLine = contents[c]; + + // If the line is a boatload of slashes, we're in the right place. + if (/^\/\/\/{6,}$/.test(sourceLine)) { + + // If 4 lines back is the same exact line, we've found the framing. + if (contents[c - 4] === sourceLine) { + + // So in that case, 2 lines back is the file path. + var parseErrorPath = contents[c - 2] + .substring(3) + .replace(/\s+\/\//, "") + ; + + var minError = new Error( + "UglifyJS minification error: \n\n" + + error.message + " at " + parseErrorPath + + (lineSrcLineNumber ? " line " + lineSrcLineNumber + "\n\n" : "") + + " within " + file.getPathInBundle() + " " + + lineErrorMessage + ":\n\n" + + lineSrcLineContent + "\n" + ); + + throw minError; + } + } + } + } + } + var allJs = ''; files.forEach(function (file) { // Don't reminify *.min.js. if (/\.min\.js$/.test(file.getPathInBundle())) { allJs += file.getContentsAsString(); } else { - allJs += UglifyJSMinify(file.getContentsAsString(), minifyOptions).code; + var minified; + try { + minified = UglifyJSMinify(file.getContentsAsString(), minifyOptions); + if (! minified.code) { + throw new Error(); + } + } catch (err) { + var filePath = file.getPathInBundle(); + + // Try to catch the ugly Uglify error. + maybeThrowMinifyErrorBySourceFile(err, file); + + err.message += " while minifying " + filePath; + throw err; + } + + allJs += minified.code; } allJs += '\n\n'; @@ -49,5 +146,3 @@ UglifyJSMinifier.prototype.processFilesForBundle = function (files, options) { files[0].addJavaScript({ data: allJs }); } }; - - From aa111f83f1287e67c30b00241fe4258dc26ff039 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Wed, 22 Feb 2017 02:57:57 +0200 Subject: [PATCH 2/9] Remove `underscore` dependency which is not used in this package. --- packages/minifier-js/package.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/minifier-js/package.js b/packages/minifier-js/package.js index be89e758e0..0c5fba2a14 100644 --- a/packages/minifier-js/package.js +++ b/packages/minifier-js/package.js @@ -12,7 +12,6 @@ Npm.strip({ }); Package.onUse(function (api) { - api.use('underscore', 'server'); api.export(['UglifyJSMinify', 'UglifyJS']); api.addFiles(['minifier.js'], 'server'); }); From e898c3a25f895e6e69b66651999804abf42c5c63 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Wed, 22 Feb 2017 02:59:39 +0200 Subject: [PATCH 3/9] Bump package versions in preparation for publishing. --- packages/minifier-js/package.js | 2 +- packages/standard-minifier-js/package.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/minifier-js/package.js b/packages/minifier-js/package.js index 0c5fba2a14..b741592858 100644 --- a/packages/minifier-js/package.js +++ b/packages/minifier-js/package.js @@ -1,6 +1,6 @@ Package.describe({ summary: "JavaScript minifier", - version: "1.2.17" + version: "1.2.18" }); Npm.depends({ diff --git a/packages/standard-minifier-js/package.js b/packages/standard-minifier-js/package.js index 94ec659c3a..8c0f4ad725 100644 --- a/packages/standard-minifier-js/package.js +++ b/packages/standard-minifier-js/package.js @@ -1,6 +1,6 @@ Package.describe({ name: 'standard-minifier-js', - version: '1.2.2', + version: '1.2.3', summary: 'Standard javascript minifiers used with Meteor apps by default.', documentation: 'README.md' }); From 4e6d07d2a3a3ea8c93aff806d2a84c84faf3a80d Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Fri, 24 Feb 2017 20:59:26 +0200 Subject: [PATCH 4/9] Undangle dangling semi-colon. --- packages/standard-minifier-js/plugin/minify-js.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/standard-minifier-js/plugin/minify-js.js b/packages/standard-minifier-js/plugin/minify-js.js index 37a4038b4d..7b58258e86 100644 --- a/packages/standard-minifier-js/plugin/minify-js.js +++ b/packages/standard-minifier-js/plugin/minify-js.js @@ -94,8 +94,7 @@ UglifyJSMinifier.prototype.processFilesForBundle = function (files, options) { // So in that case, 2 lines back is the file path. var parseErrorPath = contents[c - 2] .substring(3) - .replace(/\s+\/\//, "") - ; + .replace(/\s+\/\//, ""); var minError = new Error( "UglifyJS minification error: \n\n" + From 6c4507f004c53c44df668e20c8319510797566d8 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Fri, 24 Feb 2017 20:59:57 +0200 Subject: [PATCH 5/9] Allow `minified.code` to be an empty string. In order to allow for a blank `app.js` which occurs in the case of a Meteor app using a fully-"package"-based structure with no actual application code in the top-level. See meteor/meteor#8414 for more. --- packages/standard-minifier-js/plugin/minify-js.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/standard-minifier-js/plugin/minify-js.js b/packages/standard-minifier-js/plugin/minify-js.js index 7b58258e86..c1ff32c96b 100644 --- a/packages/standard-minifier-js/plugin/minify-js.js +++ b/packages/standard-minifier-js/plugin/minify-js.js @@ -121,7 +121,7 @@ UglifyJSMinifier.prototype.processFilesForBundle = function (files, options) { var minified; try { minified = UglifyJSMinify(file.getContentsAsString(), minifyOptions); - if (! minified.code) { + if (!(minified && typeof minified.code === "string")) { throw new Error(); } } catch (err) { From c24cb71c114223998d634b652c25b877f9764d56 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 28 Feb 2017 13:39:16 +0200 Subject: [PATCH 6/9] Bump package versions in preparation for publishing. --- packages/minifier-js/package.js | 2 +- packages/standard-minifier-js/package.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/minifier-js/package.js b/packages/minifier-js/package.js index b741592858..fc972d3041 100644 --- a/packages/minifier-js/package.js +++ b/packages/minifier-js/package.js @@ -1,6 +1,6 @@ Package.describe({ summary: "JavaScript minifier", - version: "1.2.18" + version: "1.2.18-beta.0" }); Npm.depends({ diff --git a/packages/standard-minifier-js/package.js b/packages/standard-minifier-js/package.js index 8c0f4ad725..1f0cc08f61 100644 --- a/packages/standard-minifier-js/package.js +++ b/packages/standard-minifier-js/package.js @@ -1,6 +1,6 @@ Package.describe({ name: 'standard-minifier-js', - version: '1.2.3', + version: '1.2.3-beta.0', summary: 'Standard javascript minifiers used with Meteor apps by default.', documentation: 'README.md' }); From 394812df15d5d32e2db459579a216415a6203c77 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 28 Feb 2017 14:29:46 +0200 Subject: [PATCH 7/9] Add version constraint on `minifier-js@1.2.18-beta.0`. --- packages/standard-minifier-js/package.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/standard-minifier-js/package.js b/packages/standard-minifier-js/package.js index 1f0cc08f61..fe5b1c17f7 100644 --- a/packages/standard-minifier-js/package.js +++ b/packages/standard-minifier-js/package.js @@ -8,7 +8,7 @@ Package.describe({ Package.registerBuildPlugin({ name: "minifyStdJS", use: [ - 'minifier-js' + 'minifier-js@1.2.18-beta.0' ], sources: [ 'plugin/minify-js.js' From aeee279d6d6d5324f053788d904538953bf237d9 Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 28 Feb 2017 14:37:51 +0200 Subject: [PATCH 8/9] Remove `-beta.*` suffixes in preparation for publishing. --- packages/minifier-js/package.js | 2 +- packages/standard-minifier-js/package.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/minifier-js/package.js b/packages/minifier-js/package.js index fc972d3041..b741592858 100644 --- a/packages/minifier-js/package.js +++ b/packages/minifier-js/package.js @@ -1,6 +1,6 @@ Package.describe({ summary: "JavaScript minifier", - version: "1.2.18-beta.0" + version: "1.2.18" }); Npm.depends({ diff --git a/packages/standard-minifier-js/package.js b/packages/standard-minifier-js/package.js index fe5b1c17f7..736b1d27d1 100644 --- a/packages/standard-minifier-js/package.js +++ b/packages/standard-minifier-js/package.js @@ -1,6 +1,6 @@ Package.describe({ name: 'standard-minifier-js', - version: '1.2.3-beta.0', + version: '1.2.3', summary: 'Standard javascript minifiers used with Meteor apps by default.', documentation: 'README.md' }); @@ -8,7 +8,7 @@ Package.describe({ Package.registerBuildPlugin({ name: "minifyStdJS", use: [ - 'minifier-js@1.2.18-beta.0' + 'minifier-js@1.2.18' ], sources: [ 'plugin/minify-js.js' From fc606016e21195617a64386377d9b8c45f0daecc Mon Sep 17 00:00:00 2001 From: Jesse Rosenberger Date: Tue, 28 Feb 2017 14:46:20 +0200 Subject: [PATCH 9/9] Add `History.md` entry for `standard-minifier-js` changes. (#8414) --- History.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/History.md b/History.md index 177d069fe5..fc8b2fa686 100644 --- a/History.md +++ b/History.md @@ -1,5 +1,10 @@ ## v.NEXT +* The `standard-minifier-js` and `minifier-js` packages now have improved + error capturing to provide more information on otherwise unhelpful errors + thrown when UglifyJS encounters ECMAScript grammar it is not familiar with. + [#8414](https://github.com/meteor/meteor/pull/8414) + ## v1.4.3.1, 2017-02-14 * The `meteor-babel` npm package has been upgraded to version 0.14.4,