From 75c14e3674e90a9b67335b896c39d3618fb37ac1 Mon Sep 17 00:00:00 2001 From: Jean Lauliac Date: Mon, 6 Feb 2017 10:54:38 -0800 Subject: [PATCH] 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 --- packager/src/Server/__tests__/Server-test.js | 5 +++ packager/src/Server/index.js | 37 ++++++++++---------- 2 files changed, 23 insertions(+), 19 deletions(-) diff --git a/packager/src/Server/__tests__/Server-test.js b/packager/src/Server/__tests__/Server-test.js index bcfb2cfce..5e32f529c 100644 --- a/packager/src/Server/__tests__/Server-test.js +++ b/packager/src/Server/__tests__/Server-test.js @@ -67,6 +67,7 @@ describe('processRequest', () => { beforeEach(() => { Bundler.prototype.bundle = jest.fn(() => Promise.resolve({ + getModules: () => [], getSource: () => 'this is the source', getSourceMap: () => {}, getSourceMapString: () => 'this is the source map', @@ -227,6 +228,7 @@ describe('processRequest', () => { bundleFunc .mockReturnValueOnce( Promise.resolve({ + getModules: () => [], getSource: () => 'this is the first source', getSourceMap: () => {}, getSourceMapString: () => 'this is the source map', @@ -235,6 +237,7 @@ describe('processRequest', () => { ) .mockReturnValue( Promise.resolve({ + getModules: () => [], getSource: () => 'this is the rebuilt source', getSourceMap: () => {}, getSourceMapString: () => 'this is the source map', @@ -277,6 +280,7 @@ describe('processRequest', () => { bundleFunc .mockReturnValueOnce( Promise.resolve({ + getModules: () => [], getSource: () => 'this is the first source', getSourceMap: () => {}, getSourceMapString: () => 'this is the source map', @@ -285,6 +289,7 @@ describe('processRequest', () => { ) .mockReturnValue( Promise.resolve({ + getModules: () => [], getSource: () => 'this is the rebuilt source', getSourceMap: () => {}, getSourceMapString: () => 'this is the source map', diff --git a/packager/src/Server/index.js b/packager/src/Server/index.js index f36734371..f8e43c69a 100644 --- a/packager/src/Server/index.js +++ b/packager/src/Server/index.js @@ -309,26 +309,25 @@ class Server { } const opts = bundleOpts(options); - const building = this._bundler.bundle(opts); - building.then(bundle => { - const modules = bundle.getModules(); - const nonVirtual = modules.filter(m => !m.virtual); - bundleDeps.set(bundle, { - files: new Map( - nonVirtual - .map(({sourcePath, meta = {dependencies: []}}) => - [sourcePath, meta.dependencies]) - ), - idToIndex: new Map(modules.map(({id}, i) => [id, i])), - dependencyPairs: new Map( - nonVirtual - .filter(({meta}) => meta && meta.dependencyPairs) - .map(m => [m.sourcePath, m.meta.dependencyPairs]) - ), - outdated: new Set(), - }); + return this._bundler.bundle(opts); + }).then(bundle => { + const modules = bundle.getModules(); + const nonVirtual = modules.filter(m => !m.virtual); + bundleDeps.set(bundle, { + files: new Map( + nonVirtual + .map(({sourcePath, meta = {dependencies: []}}) => + [sourcePath, meta.dependencies]) + ), + idToIndex: new Map(modules.map(({id}, i) => [id, i])), + dependencyPairs: new Map( + nonVirtual + .filter(({meta}) => meta && meta.dependencyPairs) + .map(m => [m.sourcePath, m.meta.dependencyPairs]) + ), + outdated: new Set(), }); - return building; + return bundle; }); }