From 55f4e55109768d377e38a66542bbbaefb243331b Mon Sep 17 00:00:00 2001 From: Christoph Pojer Date: Tue, 2 Feb 2016 18:09:45 -0800 Subject: [PATCH] Move process.exit outside of DependencyResolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary: node-haste shouldn't ever call process.exit and should leave it up to clients to shut down properly. This change just moves it out into the `Resolver` class – I'll leave it up to David and Martin to improve error handling for the rn packager :) public Reviewed By: davidaurelio Differential Revision: D2889908 fb-gh-sync-id: 6f03162c44d89e268891ef71c8db784a6f2e081d --- .../__tests__/DependencyGraph-test.js | 14 +++----------- .../DependencyResolver/DependencyGraph/index.js | 17 +++++++++++------ .../src/Resolver/__tests__/Resolver-test.js | 3 ++- packager/react-packager/src/Resolver/index.js | 5 +++++ 4 files changed, 21 insertions(+), 18 deletions(-) diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js index 66d02729a..09a445437 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/__tests__/DependencyGraph-test.js @@ -1189,22 +1189,14 @@ describe('DependencyGraph', function() { }, }); - const _exit = process.exit; - const _error = console.error; - - process.exit = jest.genMockFn(); - console.error = jest.genMockFn(); - var dgraph = new DependencyGraph({ ...defaults, roots: [root], }); - return dgraph.load().catch(() => { - expect(process.exit).toBeCalledWith(1); - expect(console.error).toBeCalled(); - process.exit = _exit; - console.error = _error; + return dgraph.load().catch(err => { + expect(err.message).toEqual('Failed to build DependencyGraph: Naming collision detected: /root/b.js collides with /root/index.js'); + expect(err.type).toEqual('DependencyGraphError'); }); }); diff --git a/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js b/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js index bc5d85d78..3c5ade661 100644 --- a/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js +++ b/packager/react-packager/src/DependencyResolver/DependencyGraph/index.js @@ -22,6 +22,8 @@ const ResolutionResponse = require('./ResolutionResponse'); const HasteMap = require('./HasteMap'); const DeprecatedAssetMap = require('./DeprecatedAssetMap'); +const ERROR_BUILDING_DEP_GRAPH = 'DependencyGraphError'; + const defaultActivity = { startEvent: () => {}, endEvent: () => {}, @@ -63,11 +65,7 @@ class DependencyGraph { }; this._cache = cache; this._helpers = new DependencyGraphHelpers(this._opts); - this.load().catch((err) => { - // This only happens at initialization. Live errors are easier to recover from. - console.error('Error building DependencyGraph:\n', err.stack); - process.exit(1); - }); + this.load(); } load() { @@ -134,7 +132,14 @@ class DependencyGraph { this._deprecatedAssetMap.build(), ]).then(() => activity.endEvent(depGraphActivity) - ); + ).catch(err => { + const error = new Error( + `Failed to build DependencyGraph: ${err.message}` + ); + error.type = ERROR_BUILDING_DEP_GRAPH; + error.stack = err.stack; + throw error; + }); return this._loading; } diff --git a/packager/react-packager/src/Resolver/__tests__/Resolver-test.js b/packager/react-packager/src/Resolver/__tests__/Resolver-test.js index 962bba968..7ef335d9b 100644 --- a/packager/react-packager/src/Resolver/__tests__/Resolver-test.js +++ b/packager/react-packager/src/Resolver/__tests__/Resolver-test.js @@ -10,7 +10,6 @@ jest.dontMock('../') .dontMock('underscore') - .dontMock('PixelRatio') .dontMock('../../DependencyResolver/lib/extractRequires') .dontMock('../../DependencyResolver/lib/replacePatterns'); @@ -33,6 +32,8 @@ describe('Resolver', function() { path.join.mockImpl(function(a, b) { return b; }); + + DependencyGraph.prototype.load.mockImpl(() => Promise.resolve()); }); class ResolutionResponseMock { diff --git a/packager/react-packager/src/Resolver/index.js b/packager/react-packager/src/Resolver/index.js index 1822eb994..4c6e3efbd 100644 --- a/packager/react-packager/src/Resolver/index.js +++ b/packager/react-packager/src/Resolver/index.js @@ -103,6 +103,11 @@ class Resolver { }); this._polyfillModuleNames = opts.polyfillModuleNames || []; + + this._depGraph.load().catch(err => { + console.error(err.message + '\n' + err.stack); + process.exit(1); + }); } getShallowDependencies(entryFile) {