Skip to content

Commit 77bfdb7

Browse files
mikrostewmikrostew
committed
fix error handling and add tests
1 parent 70b69be commit 77bfdb7

File tree

5 files changed

+134
-57
lines changed

5 files changed

+134
-57
lines changed

‎index.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,12 +107,18 @@ UglifyWriter.prototype.build = function () {
107107
// files are finished processing, shut down the workers
108108
writer.pool.terminate();
109109
return writer.outputPath;
110+
})
111+
.catch((e) => {
112+
// make sure to shut down the workers on error
113+
writer.pool.terminate();
114+
throw e;
110115
});
111116
};
112117

113118
UglifyWriter.prototype.processFile = function(inFile, outFile, relativePath, outDir) {
114119
// don't run this in the workerpool if concurrency is disabled (can set JOBS <= 1)
115120
if (this.async && this.concurrency > 1) {
121+
// each of these arguments is a string, which can be sent to the worker process as-is
116122
return this.pool.exec('processFileParallel', [inFile, outFile, relativePath, outDir, silent, this.options]);
117123
}
118124
return processFile(inFile, outFile, relativePath, outDir, silent, this.options);

‎lib/process-file.js‎

Lines changed: 51 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -11,73 +11,67 @@ var Promise = require('rsvp').Promise;
1111
var UglifyJS = require('uglify-es');
1212

1313

14-
// each of these is a string, so that's good
1514
module.exports = function processFile(inFile, outFile, relativePath, outDir, silent, _options) {
16-
return new Promise((resolve, reject) => {
15+
var src = fs.readFileSync(inFile, 'utf-8');
16+
var mapName = path.basename(outFile).replace(/\.js$/,'') + '.map';
1717

18-
var src = fs.readFileSync(inFile, 'utf-8');
19-
var mapName = path.basename(outFile).replace(/\.js$/,'') + '.map';
18+
var mapDir;
19+
if (_options.sourceMapDir) {
20+
mapDir = path.join(outDir, _options.sourceMapDir);
21+
} else {
22+
mapDir = path.dirname(path.join(outDir, relativePath));
23+
}
2024

21-
var mapDir;
22-
if (_options.sourceMapDir) {
23-
mapDir = path.join(outDir, _options.sourceMapDir);
24-
} else {
25-
mapDir = path.dirname(path.join(outDir, relativePath));
26-
}
27-
28-
let options = defaults({}, _options.uglify);
29-
if (options.sourceMap) {
30-
let filename = path.basename(inFile);
31-
let url = _options.sourceMapDir ? '/' + path.join(_options.sourceMapDir, mapName) : mapName;
25+
let options = defaults({}, _options.uglify);
26+
if (options.sourceMap) {
27+
let filename = path.basename(inFile);
28+
let url = _options.sourceMapDir ? '/' + path.join(_options.sourceMapDir, mapName) : mapName;
3229

33-
let sourceMap = { filename, url };
30+
let sourceMap = { filename, url };
3431

35-
if (srcURL.existsIn(src)) {
36-
let url = srcURL.getFrom(src);
37-
let sourceMapPath = path.join(path.dirname(inFile), url);
38-
if (fs.existsSync(sourceMapPath)) {
39-
sourceMap.content = JSON.parse(fs.readFileSync(sourceMapPath));
40-
} else if (!silent) {
41-
console.warn(`[WARN] (broccoli-uglify-sourcemap) "${url}" referenced in "${relativePath}" could not be found`);
42-
}
32+
if (srcURL.existsIn(src)) {
33+
let url = srcURL.getFrom(src);
34+
let sourceMapPath = path.join(path.dirname(inFile), url);
35+
if (fs.existsSync(sourceMapPath)) {
36+
sourceMap.content = JSON.parse(fs.readFileSync(sourceMapPath));
37+
} else if (!silent) {
38+
console.warn(`[WARN] (broccoli-uglify-sourcemap) "${url}" referenced in "${relativePath}" could not be found`);
4339
}
44-
45-
options = defaults(options, { sourceMap });
4640
}
4741

48-
var start = new Date();
49-
debug('[starting]: %s %dKB', relativePath, (src.length / 1000));
50-
var result = UglifyJS.minify(src, options);
51-
var end = new Date();
52-
var total = end - start;
53-
if (total > 20000 && !silent) {
54-
console.warn('[WARN] (broccoli-uglify-sourcemap) Minifying: `' + relativePath + '` took: ' + total + 'ms (more than 20,000ms)');
55-
}
42+
options = defaults(options, { sourceMap });
43+
}
5644

57-
if (result.error) {
58-
result.error.filename = relativePath;
59-
reject(result.error);
60-
return;
61-
}
45+
var start = new Date();
46+
debug('[starting]: %s %dKB', relativePath, (src.length / 1000));
47+
var result = UglifyJS.minify(src, options);
48+
var end = new Date();
49+
var total = end - start;
50+
if (total > 20000 && !silent) {
51+
console.warn('[WARN] (broccoli-uglify-sourcemap) Minifying: `' + relativePath + '` took: ' + total + 'ms (more than 20,000ms)');
52+
}
6253

63-
debug('[finished]: %s %dKB in %dms', relativePath, (result.code.length / 1000), total);
54+
if (result.error) {
55+
result.error.filename = relativePath;
56+
throw result.error;
57+
}
6458

65-
if (options.sourceMap) {
66-
var newSourceMap = JSON.parse(result.map);
59+
debug('[finished]: %s %dKB in %dms', relativePath, (result.code.length / 1000), total);
6760

68-
newSourceMap.sources = newSourceMap.sources.map(function(path){
69-
// If out output file has the same name as one of our original
70-
// sources, they will shadow eachother in Dev Tools. So instead we
71-
// alter the reference to the upstream file.
72-
if (path === relativePath) {
73-
path = path.replace(/\.js$/, '-orig.js');
74-
}
75-
return path;
76-
});
77-
mkdirp.sync(mapDir);
78-
fs.writeFileSync(path.join(mapDir, mapName), JSON.stringify(newSourceMap));
79-
}
80-
fs.writeFileSync(outFile, result.code);
81-
resolve();
82-
});
61+
if (options.sourceMap) {
62+
var newSourceMap = JSON.parse(result.map);
63+
64+
newSourceMap.sources = newSourceMap.sources.map(function(path){
65+
// If out output file has the same name as one of our original
66+
// sources, they will shadow eachother in Dev Tools. So instead we
67+
// alter the reference to the upstream file.
68+
if (path === relativePath) {
69+
path = path.replace(/\.js$/, '-orig.js');
70+
}
71+
return path;
72+
});
73+
mkdirp.sync(mapDir);
74+
fs.writeFileSync(path.join(mapDir, mapName), JSON.stringify(newSourceMap));
75+
}
76+
fs.writeFileSync(outFile, result.code);
8377
};

‎test/__snapshots__/test.js.snap‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,31 @@ Object {
9898
}
9999
`;
100100

101+
exports[`broccoli-uglify-sourcemap on error rejects with BuildError 1`] = `Object {}`;
102+
103+
exports[`broccoli-uglify-sourcemap on error rejects with BuildError async 1`] = `Object {}`;
104+
105+
exports[`broccoli-uglify-sourcemap on error shuts down the workerpool 1`] = `Object {}`;
106+
107+
exports[`broccoli-uglify-sourcemap shuts down the workerpool 1`] = `
108+
Object {
109+
"inside": Object {
110+
"with-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
111+
//# sourceMappingURL=with-upstream-sourcemap.map",
112+
"with-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"/inner/first.js\\",\\"/inner/second.js\\",\\"/other/fourth.js\\",\\"/other/third.js\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AAAA,SAAAA,gBAEA,MAAA,IAAAC,MADA,IAIA,SAAAC,OACA,MAAA,IAAAD,MAAA,QCNA,SAAAE,gBACA,MAAA,IAAAF,MAAA,kBCEA,SAAAG,SACA,MAAA,IAAAH,MAAA,UCJA,SAAAI,QACA,MAAA,IAAAJ,MAAA\\",\\"file\\":\\"with-upstream-sourcemap.js\\",\\"sourcesContent\\":[\\"function meaningOfLife() {\\\\n var thisIsALongLocal = 42;\\\\n throw new Error(thisIsALongLocal);\\\\n}\\\\n\\\\nfunction boom() {\\\\n throw new Error('boom');\\\\n}\\\\n\\",\\"function somethingElse() {\\\\n throw new Error(\\\\\\"somethign else\\\\\\");\\\\n}\\\\n\\",\\"\\\\n// Hello world\\\\n\\\\nfunction fourth(){\\\\n throw new Error('fourth');\\\\n}\\\\n\\",\\"function third(){\\\\n throw new Error(\\\\\\"oh no\\\\\\");\\\\n}\\\\n\\"]}",
113+
},
114+
"no-upstream-sourcemap.js": "function meaningOfLife(){throw new Error(42)}function boom(){throw new Error(\\"boom\\")}function somethingElse(){throw new Error(\\"somethign else\\")}function fourth(){throw new Error(\\"fourth\\")}function third(){throw new Error(\\"oh no\\")}
115+
//# sourceMappingURL=no-upstream-sourcemap.map",
116+
"no-upstream-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"0\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\",\\"boom\\",\\"somethingElse\\",\\"fourth\\",\\"third\\"],\\"mappings\\":\\"AACA,SAASA,gBAEP,MAAM,IAAIC,MADa,IAIzB,SAASC,OACP,MAAM,IAAID,MAAM,QAGlB,SAASE,gBACP,MAAM,IAAIF,MAAM,kBAMlB,SAASG,SACP,MAAM,IAAIH,MAAM,UAGlB,SAASI,QACP,MAAM,IAAIJ,MAAM\\",\\"file\\":\\"no-upstream-sourcemap.js\\"}",
117+
"something.css": "#id {
118+
background: white;
119+
}",
120+
"with-broken-sourcemap.js": "function meaningOfLife(){throw new Error(42)}
121+
//# sourceMappingURL=with-broken-sourcemap.map",
122+
"with-broken-sourcemap.map": "{\\"version\\":3,\\"sources\\":[\\"0\\"],\\"names\\":[\\"meaningOfLife\\",\\"Error\\"],\\"mappings\\":\\"AAAA,SAASA,gBAEP,MAAM,IAAIC,MADa\\",\\"file\\":\\"with-broken-sourcemap.js\\"}",
123+
}
124+
`;
125+
101126
exports[`broccoli-uglify-sourcemap supports alternate sourcemap location 1`] = `
102127
Object {
103128
"inside": Object {

‎test/fixtures-error/no-upstream-sourcemap.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
/* This file has an error. */
2+
var i =
3+

‎test/test.js‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ var path = require('path');
66
var helpers = require('broccoli-test-helper');
77

88
var fixtures = path.join(__dirname, 'fixtures');
9+
var fixturesError = path.join(__dirname, 'fixtures-error');
910

1011
var createTempDir = helpers.createTempDir;
1112
var createBuilder = helpers.createBuilder;
@@ -78,6 +79,54 @@ let { bar } = Foo.prototype;`,
7879
expect(builder.read()).toMatchSnapshot();
7980
});
8081

82+
it('shuts down the workerpool', async function() {
83+
var testUglify = new uglify(fixtures, { async: true });
84+
builder = createBuilder(testUglify);
85+
86+
await builder.build();
87+
88+
expect(builder.read()).toMatchSnapshot();
89+
expect(testUglify.pool.stats().totalWorkers).toEqual(0);
90+
});
91+
92+
describe('on error', function() {
93+
it('rejects with BuildError', async function() {
94+
builder = createBuilder(new uglify(fixturesError, {}));
95+
96+
var shouldError;
97+
await builder.build()
98+
.catch(err => {
99+
shouldError = err;
100+
});
101+
expect(shouldError.name).toEqual('BuildError');
102+
103+
expect(builder.read()).toMatchSnapshot();
104+
});
105+
106+
it('rejects with BuildError async', async function() {
107+
builder = createBuilder(new uglify(fixturesError, { async: true }));
108+
109+
var shouldError;
110+
await builder.build()
111+
.catch(err => {
112+
shouldError = err;
113+
});
114+
expect(shouldError.name).toEqual('BuildError');
115+
116+
expect(builder.read()).toMatchSnapshot();
117+
});
118+
119+
it('shuts down the workerpool', async function() {
120+
var testUglify = new uglify(fixturesError, { async: true });
121+
builder = createBuilder(testUglify);
122+
123+
await builder.build().catch(err => {});
124+
125+
expect(builder.read()).toMatchSnapshot();
126+
expect(testUglify.pool.stats().totalWorkers).toEqual(0);
127+
});
128+
});
129+
81130
afterEach(async function() {
82131
if (input) {
83132
await input.dispose();

0 commit comments

Comments
 (0)