packager: fix uncaught rejection

Summary: I figured out that the uncaught rejection that happens on transform error originated from a "fork" along the chain, where we both "then" on a promise, and return that promise. In that case Node.js, legitimately I think, identifies this as an uncaught rejection case. One solution, in this changeset, is to do the side-effects as part of the promise chain instead of "forking". Another option would be to add an explicit error handler to the additional "then", but it seems we don't have to handle that case here.

Reviewed By: davidaurelio

Differential Revision: D4515592

fbshipit-source-id: 17d813cfdbc76685b6273b8d94e97d948fd68674
This commit is contained in:
Jean Lauliac 2017-02-06 10:54:38 -08:00 committed by Facebook Github Bot
parent 233015c93e
commit 75c14e3674
2 changed files with 23 additions and 19 deletions

View File

@ -67,6 +67,7 @@ describe('processRequest', () => {
beforeEach(() => { beforeEach(() => {
Bundler.prototype.bundle = jest.fn(() => Bundler.prototype.bundle = jest.fn(() =>
Promise.resolve({ Promise.resolve({
getModules: () => [],
getSource: () => 'this is the source', getSource: () => 'this is the source',
getSourceMap: () => {}, getSourceMap: () => {},
getSourceMapString: () => 'this is the source map', getSourceMapString: () => 'this is the source map',
@ -227,6 +228,7 @@ describe('processRequest', () => {
bundleFunc bundleFunc
.mockReturnValueOnce( .mockReturnValueOnce(
Promise.resolve({ Promise.resolve({
getModules: () => [],
getSource: () => 'this is the first source', getSource: () => 'this is the first source',
getSourceMap: () => {}, getSourceMap: () => {},
getSourceMapString: () => 'this is the source map', getSourceMapString: () => 'this is the source map',
@ -235,6 +237,7 @@ describe('processRequest', () => {
) )
.mockReturnValue( .mockReturnValue(
Promise.resolve({ Promise.resolve({
getModules: () => [],
getSource: () => 'this is the rebuilt source', getSource: () => 'this is the rebuilt source',
getSourceMap: () => {}, getSourceMap: () => {},
getSourceMapString: () => 'this is the source map', getSourceMapString: () => 'this is the source map',
@ -277,6 +280,7 @@ describe('processRequest', () => {
bundleFunc bundleFunc
.mockReturnValueOnce( .mockReturnValueOnce(
Promise.resolve({ Promise.resolve({
getModules: () => [],
getSource: () => 'this is the first source', getSource: () => 'this is the first source',
getSourceMap: () => {}, getSourceMap: () => {},
getSourceMapString: () => 'this is the source map', getSourceMapString: () => 'this is the source map',
@ -285,6 +289,7 @@ describe('processRequest', () => {
) )
.mockReturnValue( .mockReturnValue(
Promise.resolve({ Promise.resolve({
getModules: () => [],
getSource: () => 'this is the rebuilt source', getSource: () => 'this is the rebuilt source',
getSourceMap: () => {}, getSourceMap: () => {},
getSourceMapString: () => 'this is the source map', getSourceMapString: () => 'this is the source map',

View File

@ -309,8 +309,8 @@ class Server {
} }
const opts = bundleOpts(options); const opts = bundleOpts(options);
const building = this._bundler.bundle(opts); return this._bundler.bundle(opts);
building.then(bundle => { }).then(bundle => {
const modules = bundle.getModules(); const modules = bundle.getModules();
const nonVirtual = modules.filter(m => !m.virtual); const nonVirtual = modules.filter(m => !m.virtual);
bundleDeps.set(bundle, { bundleDeps.set(bundle, {
@ -327,8 +327,7 @@ class Server {
), ),
outdated: new Set(), outdated: new Set(),
}); });
}); return bundle;
return building;
}); });
} }