Skip to content

Commit

Permalink
packager: fix uncaught rejection
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Jean Lauliac authored and normanjoyner committed Feb 9, 2017
1 parent 0e352e7 commit fbc3d4b
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 19 deletions.
5 changes: 5 additions & 0 deletions packager/src/Server/__tests__/Server-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand Down Expand Up @@ -227,6 +228,7 @@ describe('processRequest', () => {
bundleFunc
.mockReturnValueOnce(
Promise.resolve({
getModules: () => [],
getSource: () => 'this is the first source',
getSourceMap: () => {},
getSourceMapString: () => 'this is the source map',
Expand All @@ -235,6 +237,7 @@ describe('processRequest', () => {
)
.mockReturnValue(
Promise.resolve({
getModules: () => [],
getSource: () => 'this is the rebuilt source',
getSourceMap: () => {},
getSourceMapString: () => 'this is the source map',
Expand Down Expand Up @@ -277,6 +280,7 @@ describe('processRequest', () => {
bundleFunc
.mockReturnValueOnce(
Promise.resolve({
getModules: () => [],
getSource: () => 'this is the first source',
getSourceMap: () => {},
getSourceMapString: () => 'this is the source map',
Expand All @@ -285,6 +289,7 @@ describe('processRequest', () => {
)
.mockReturnValue(
Promise.resolve({
getModules: () => [],
getSource: () => 'this is the rebuilt source',
getSourceMap: () => {},
getSourceMapString: () => 'this is the source map',
Expand Down
37 changes: 18 additions & 19 deletions packager/src/Server/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
});
}

Expand Down

0 comments on commit fbc3d4b

Please sign in to comment.