Merge branch 'spalger-notify-browser-on-error'

parcel-bundler · Dec 8, 2017 · d9d8bab · d9d8bab
2 parents 582f8db + ed5982a
commit d9d8bab
Show file tree

Hide file tree

Showing 8 changed files with 199 additions and 23 deletions.
diff --git a/src/Bundler.js b/src/Bundler.js
@@ -133,6 +133,9 @@ class Bundler extends EventEmitter {
     } catch (err) {
       this.errored = true;
       this.logger.error(err);
+      if (this.hmr) {
+        this.hmr.emitError(err);
+      }
     } finally {
       this.pending = false;
       this.emit('buildEnd');

diff --git a/src/HMRServer.js b/src/HMRServer.js
@@ -1,20 +1,50 @@
 const WebSocket = require('ws');
+const prettyError = require('./utils/prettyError');
 
 class HMRServer {
   async start() {
     await new Promise((resolve) => {
       this.wss = new WebSocket.Server({port: 0}, resolve);
     });
 
+    this.wss.on('connection', (ws) => {
+      if (this.unresolvedError) {
+        ws.send(JSON.stringify(this.unresolvedError))
+      }
+    });
+
     return this.wss._server.address().port;
   }
 
   stop() {
     this.wss.close();
   }
 
+  emitError(err) {
+    let {message, stack} = prettyError(err);
+
+    // store the most recent error so we can notify new connections
+    // and so we can broadcast when the error is resolved
+    this.unresolvedError = {
+      type: 'error',
+      error: {
+        message,
+        stack
+      }
+    };
+
+    this.broadcast(this.unresolvedError)
+  }
+
   emitUpdate(assets) {
-    let msg = JSON.stringify({
+    if (this.unresolvedError) {
+      this.unresolvedError = null
+      this.broadcast({
+        type: 'error-resolved'
+      });
+    }
+
+    this.broadcast({
       type: 'update',
       assets: assets.map(asset => {
         let deps = {};
@@ -30,9 +60,12 @@ class HMRServer {
         };
       })
     });
+  }
 
+  broadcast(msg) {
+    const json = JSON.stringify(msg)
     for (let ws of this.wss.clients) {
-      ws.send(msg);
+      ws.send(json);
     }
   }
 }

diff --git a/src/Logger.js b/src/Logger.js
@@ -1,5 +1,6 @@
 const chalk = require('chalk');
 const readline = require('readline');
+const prettyError = require('./utils/prettyError');
 
 class Logger {
   constructor(options) {
@@ -47,26 +48,11 @@ class Logger {
       return;
     }
 
-    let message = typeof err === 'string' ? err : err.message;
-    if (!message) {
-      return;
-    }
-
-    if (err.fileName) {
-      let fileName = err.fileName;
-      if (err.loc) {
-        fileName += `:${err.loc.line}:${err.loc.column}`;
-      }
-
-      message = `${fileName}: ${message}`;
-    }
+    let {message, stack} = prettyError(err, {color: this.color});
 
     this.status('🚨', message, 'red');
-
-    if (err.codeFrame) {
-      this.write((this.color && err.highlightedCodeFrame) || err.codeFrame);
-    } else if (err.stack) {
-      this.write(err.stack.slice(err.stack.indexOf('\n') + 1));
+    if (stack) {
+      this.write(stack);
     }
   }
 

diff --git a/src/Server.js b/src/Server.js
@@ -15,8 +15,10 @@ function middleware(bundler) {
     }
 
     function respond() {
-      // If the URL doesn't start with the public path, send the main HTML bundle
-      if (!req.url.startsWith(bundler.options.publicURL)) {
+      if (bundler.errored) {
+        return send500();
+      } else if (!req.url.startsWith(bundler.options.publicURL)) {
+        // If the URL doesn't start with the public path, send the main HTML bundle
         return sendIndex();
       } else {
         // Otherwise, serve the file from the dist folder
@@ -35,6 +37,12 @@ function middleware(bundler) {
       }
     }
 
+    function send500() {
+      res.setHeader('Content-Type', 'text/plain; charset=utf-8');
+      res.writeHead(500);
+      res.end('🚨 Build error, check the console for details.');
+    }
+
     function send404() {
       if (next) {
         return next();

diff --git a/src/builtins/hmr-runtime.js b/src/builtins/hmr-runtime.js
@@ -30,6 +30,14 @@ if (!module.bundle.parent) {
         }
       }
     }
+
+    if (data.type === 'error-resolved') {
+      console.log('[parcel] ✨ Error resolved');
+    }
+
+    if (data.type === 'error') {
+      console.error(`[parcel] 🚨 ${data.error.message}\n${data.error.stack}`);
+    }
   };
 }
 

diff --git a/src/utils/prettyError.js b/src/utils/prettyError.js
@@ -0,0 +1,24 @@
+module.exports = function (err, opts = {}) {
+  let message = typeof err === 'string' ? err : err.message;
+  if (!message) {
+    message = 'Unknown error';
+  }
+
+  if (err.fileName) {
+    let fileName = err.fileName;
+    if (err.loc) {
+      fileName += `:${err.loc.line}:${err.loc.column}`;
+    }
+
+    message = `${fileName}: ${message}`;
+  }
+
+  let stack;
+  if (err.codeFrame) {
+    stack = (opts.color && err.highlightedCodeFrame) || err.codeFrame;
+  } else if (err.stack) {
+    stack = err.stack.slice(err.stack.indexOf('\n') + 1);
+  }
+
+  return {message, stack};
+};
diff --git a/test/hmr.js b/test/hmr.js
@@ -67,6 +67,56 @@ describe('hmr', function () {
     assert.equal(msg.assets.length, 2);
   });
 
+  it('should emit an HMR error on bundle failure', async function () {
+    await ncp(__dirname + '/integration/commonjs', __dirname + '/input');
+
+    b = bundler(__dirname + '/input/index.js', {watch: true, hmr: true});
+    let bundle = await b.bundle();
+
+    ws = new WebSocket('ws://localhost:' + b.options.hmrPort);
+
+    fs.writeFileSync(__dirname + '/input/local.js', 'require("fs"; exports.a = 5; exports.b = 5;');
+
+    let msg = JSON.parse(await nextEvent(ws, 'message'));
+    assert.equal(msg.type, 'error');
+    assert.equal(msg.error.message, __dirname + '/input/local.js:1:12: Unexpected token, expected , (1:12)');
+    assert.equal(msg.error.stack, '> 1 | require("fs"; exports.a = 5; exports.b = 5;\n    |             ^');
+  });
+
+  it('should emit an HMR error to new connections after a bundle failure', async function () {
+    await ncp(__dirname + '/integration/commonjs', __dirname + '/input');
+
+    b = bundler(__dirname + '/input/index.js', {watch: true, hmr: true});
+    let bundle = await b.bundle();
+
+    fs.writeFileSync(__dirname + '/input/local.js', 'require("fs"; exports.a = 5; exports.b = 5;');
+    await nextEvent(b, 'buildEnd');
+    await sleep(50);
+
+    ws = new WebSocket('ws://localhost:' + b.options.hmrPort);
+    let msg = JSON.parse(await nextEvent(ws, 'message'));
+    assert.equal(msg.type, 'error');
+  });
+
+  it('should emit an HMR error-resolved on build after error', async function () {
+    await ncp(__dirname + '/integration/commonjs', __dirname + '/input');
+
+    b = bundler(__dirname + '/input/index.js', {watch: true, hmr: true});
+    let bundle = await b.bundle();
+
+    ws = new WebSocket('ws://localhost:' + b.options.hmrPort);
+
+    fs.writeFileSync(__dirname + '/input/local.js', 'require("fs"; exports.a = 5; exports.b = 5;');
+
+    let msg = JSON.parse(await nextEvent(ws, 'message'));
+    assert.equal(msg.type, 'error');
+
+    fs.writeFileSync(__dirname + '/input/local.js', 'require("fs"); exports.a = 5; exports.b = 5;');
+
+    let msg2 = JSON.parse(await nextEvent(ws, 'message'));
+    assert.equal(msg2.type, 'error-resolved');
+  });
+
   it('should accept HMR updates in the runtime', async function () {
     await ncp(__dirname + '/integration/hmr', __dirname + '/input');
 
@@ -131,4 +181,51 @@ describe('hmr', function () {
     await sleep(50);
     assert.deepEqual(outputs, [3, 10]);
   });
-});
+
+  it('should log emitted errors', async function () {
+    await ncp(__dirname + '/integration/commonjs', __dirname + '/input');
+
+    b = bundler(__dirname + '/input/index.js', {watch: true, hmr: true});
+    let bundle = await b.bundle();
+
+    let logs = [];
+    run(bundle, {
+      console: {
+        error(msg) { logs.push(msg) },
+      }
+    });
+
+    fs.writeFileSync(__dirname + '/input/local.js', 'require("fs"; exports.a = 5; exports.b = 5;');
+    await nextEvent(b, 'buildEnd');
+    await sleep(50);
+
+    assert.equal(logs.length, 1)
+    assert(logs[0].trim().startsWith('[parcel] 🚨'));
+  });
+
+  it('should log when errors resolve', async function () {
+    await ncp(__dirname + '/integration/commonjs', __dirname + '/input');
+
+    b = bundler(__dirname + '/input/index.js', {watch: true, hmr: true});
+    let bundle = await b.bundle();
+
+    let logs = [];
+    run(bundle, {
+      console: {
+        error(msg) { logs.push(msg) },
+        log(msg) { logs.push(msg) },
+      }
+    });
+
+    fs.writeFileSync(__dirname + '/input/local.js', 'require("fs"; exports.a = 5; exports.b = 5;');
+    await nextEvent(b, 'buildEnd');
+
+    fs.writeFileSync(__dirname + '/input/local.js', 'require("fs"); exports.a = 5; exports.b = 5;');
+    await nextEvent(b, 'buildEnd');
+    await sleep(50);
+
+    assert.equal(logs.length, 2)
+    assert(logs[0].trim().startsWith('[parcel] 🚨'));
+    assert(logs[1].trim().startsWith('[parcel] ✨'));
+  });
+});
diff --git a/test/server.js b/test/server.js
@@ -61,4 +61,21 @@ describe('server', function () {
 
     assert(threw);
   });
+
+  it('should serve a 500 if the bundler errored', async function () {
+    let b = bundler(__dirname + '/integration/html/index.html');
+    server = b.serve(0);
+
+    b.errored = true;
+
+    try {
+      await get('/');
+      throw new Error('GET / responded with 200')
+    } catch (err) {
+      assert.equal(err.message, 'Request failed: 500');
+    }
+
+    b.errored = false;
+    await get('/');
+  });
 });