Make DeltaBundler HMR handle errors correctly

Reviewed By: jeanlauliac

Differential Revision: D5814107

fbshipit-source-id: 2bcc52901eff5f2330453c7dc948a0b4ac0332db

packages/metro-bundler/src/HmrServer/index.js

@@ -12,6 +12,7 @@
 
 'use strict';
 
+const formatBundlingError = require('../lib/formatBundlingError');
 const getBundlingOptionsForHmr = require('./getBundlingOptionsForHmr');
 const querystring = require('querystring');
 const url = require('url');
@@ -92,7 +93,17 @@ class HmrServer<TClient: Client> {
   }
 
   async _prepareResponse(client: Client): Promise<{type: string, body: {}}> {
-    const result = await client.deltaTransformer.getDelta();
+    let result;
+
+    try {
+      result = await client.deltaTransformer.getDelta();
+    } catch (error) {
+      const formattedError = formatBundlingError(error);
+
+      this._reporter.update({type: 'bundling_error', error});
+
+      return {type: 'error', body: formattedError};
+    }
     const modules = [];
 
     for (const [id, module] of result.delta) {

packages/metro-bundler/src/Server/index.js

@@ -21,6 +21,7 @@ const crypto = require('crypto');
 const debug = require('debug')('Metro:Server');
 const defaults = require('../defaults');
 const emptyFunction = require('fbjs/lib/emptyFunction');
+const formatBundlingError = require('../lib/formatBundlingError');
 const getMaxWorkers = require('../lib/getMaxWorkers');
 const mime = require('mime-types');
 const parsePlatformFilePath = require('../node-haste/lib/parsePlatformFilePath');
@@ -28,10 +29,7 @@ const path = require('path');
 const symbolicate = require('./symbolicate');
 const url = require('url');
 
-const {
+import type {CustomError} from '../lib/formatBundlingError';
-  AmbiguousModuleResolutionError,
-} = require('../node-haste/DependencyGraph/ResolutionRequest');
-
 import type Module, {HasteImpl} from '../node-haste/Module';
 import type {IncomingMessage, ServerResponse} from 'http';
 import type ResolutionResponse from '../node-haste/DependencyGraph/ResolutionResponse';
@@ -1093,72 +1091,20 @@ class Server {
     );
   }
 
-  _handleError(
+  _handleError(res: ServerResponse, bundleID: string, error: CustomError) {
-    res: ServerResponse,
-    bundleID: string,
-    error: {
-      status: number,
-      type: string,
-      description: string,
-      filename: string,
-      lineNumber: number,
-      errors: Array<{
-        description: string,
-        filename: string,
-        lineNumber: number,
-      }>,
-    },
-  ) {
     res.writeHead(error.status || 500, {
       'Content-Type': 'application/json; charset=UTF-8',
     });
 
-    if (error instanceof AmbiguousModuleResolutionError) {
+    const formattedError = formatBundlingError(error);
-      const he = error.hasteError;
+
-      const message =
+    res.end(JSON.stringify(formattedError));
-        "Ambiguous resolution: module '" +
+
-        `${error.fromModulePath}\' tries to require \'${he.hasteName}\', but ` +
+    if (error instanceof Error && error.type === 'NotFoundError') {
-        `there are several files providing this module. You can delete or ` +
+      delete this._bundles[bundleID];
-        'fix them: \n\n' +
-        Object.keys(he.duplicatesSet)
-          .sort()
-          .map(dupFilePath => `${dupFilePath}`)
-          .join('\n\n');
-      res.end(JSON.stringify({message, errors: [{description: message}]}));
-      this._reporter.update({error, type: 'bundling_error'});
-      return;
     }
 
-    if (
+    this._reporter.update({error, type: 'bundling_error'});
-      error instanceof Error &&
-      (error.type === 'TransformError' ||
-        error.type === 'NotFoundError' ||
-        error.type === 'UnableToResolveError')
-    ) {
-      error.errors = [
-        {
-          description: error.description,
-          filename: error.filename,
-          lineNumber: error.lineNumber,
-        },
-      ];
-      res.end(JSON.stringify(error));
-
-      if (error.type === 'NotFoundError') {
-        delete this._bundles[bundleID];
-      }
-      this._reporter.update({error, type: 'bundling_error'});
-    } else {
-      console.error(error.stack || error);
-      res.end(
-        JSON.stringify({
-          type: 'InternalError',
-          message:
-            'Metro Bundler has encountered an internal error, ' +
-            'please check your terminal error output for more details',
-        }),
-      );
-    }
   }
 
   _getOptionsFromUrl(reqUrl: string): BundleOptions {

packages/metro-bundler/src/lib/formatBundlingError.js

@@ -0,0 +1,80 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ *
+ * @format
+ * @flow
+ */
+
+'use strict';
+
+const {
+  AmbiguousModuleResolutionError,
+} = require('../node-haste/DependencyGraph/ResolutionRequest');
+
+export type CustomError = Error & {|
+  status?: number,
+  type?: string,
+  description?: string,
+  filename?: string,
+  lineNumber?: number,
+  errors?: Array<{
+    description: string,
+    filename: string,
+    lineNumber: number,
+  }>,
+|};
+
+function formatBundlingError(
+  error: CustomError,
+): {type: string, message: string, errors: Array<{description: string}>} {
+  if (error instanceof AmbiguousModuleResolutionError) {
+    const he = error.hasteError;
+    const message =
+      "Ambiguous resolution: module '" +
+      `${error.fromModulePath}\' tries to require \'${he.hasteName}\', but ` +
+      `there are several files providing this module. You can delete or ` +
+      'fix them: \n\n' +
+      Object.keys(he.duplicatesSet)
+        .sort()
+        .map(dupFilePath => `${dupFilePath}`)
+        .join('\n\n');
+
+    return {
+      type: 'AmbiguousModuleResolutionError',
+      message,
+      errors: [{description: message}],
+    };
+  }
+
+  if (
+    error instanceof Error &&
+    (error.type === 'TransformError' ||
+      error.type === 'NotFoundError' ||
+      error.type === 'UnableToResolveError')
+  ) {
+    error.errors = [
+      {
+        description: error.description,
+        filename: error.filename,
+        lineNumber: error.lineNumber,
+      },
+    ];
+
+    return error;
+  } else {
+    return {
+      type: 'InternalError',
+      errors: [],
+      message:
+        'Metro Bundler has encountered an internal error, ' +
+        'please check your terminal error output for more details',
+    };
+  }
+}
+
+module.exports = formatBundlingError;