From ba1c673a07e255d7bdeadffd199bf7ca9fbba72e Mon Sep 17 00:00:00 2001
From: Kees Kluskens <kees@webduck.nl>
Date: Sat, 10 Dec 2016 23:14:57 +0100
Subject: [PATCH] Fix error handling in compiler `watch()` and `run()`

This changes it to be more like how webpack's watch mode handles it;
if the compiler throws an error in `watch()` or `run()`, the middleware
should continu to run instead of quitting.
---
 lib/Shared.js                  | 15 ++++++++-----
 test/CompilerCallbacks.test.js | 40 ++++++++++++++++++++++++++++++++++
 test/Lazy.test.js              | 14 +++++++-----
 3 files changed, 58 insertions(+), 11 deletions(-)
 create mode 100644 test/CompilerCallbacks.test.js

diff --git a/lib/Shared.js b/lib/Shared.js
index 93792e269..ef70df256 100644
--- a/lib/Shared.js
+++ b/lib/Shared.js
@@ -11,6 +11,7 @@ module.exports = function Shared(context) {
 			if(typeof options.reporter !== "function") options.reporter = share.defaultReporter;
 			if(typeof options.log !== "function") options.log = console.log.bind(console);
 			if(typeof options.warn !== "function") options.warn = console.warn.bind(console);
+			if(typeof options.error !== "function") options.error = console.error.bind(console);
 			if(typeof options.watchDelay !== "undefined") {
 				// TODO remove this in next major version
 				options.warn("options.watchDelay is deprecated: Use 'options.watchOptions.aggregateTimeout' instead");
@@ -149,9 +150,7 @@ module.exports = function Shared(context) {
 			var compiler = context.compiler;
 			// start watching
 			if(!options.lazy) {
-				var watching = compiler.watch(options.watchOptions, function(err) {
-					if(err) throw err;
-				});
+				var watching = compiler.watch(options.watchOptions, share.handleCompilerCallback);
 				context.watching = watching;
 			} else {
 				context.state = true;
@@ -160,13 +159,17 @@ module.exports = function Shared(context) {
 		rebuild: function rebuild() {
 			if(context.state) {
 				context.state = false;
-				context.compiler.run(function(err) {
-					if(err) throw err;
-				});
+				context.compiler.run(share.handleCompilerCallback);
 			} else {
 				context.forceRebuild = true;
 			}
 		},
+		handleCompilerCallback: function(err) {
+			if(err) {
+				context.options.error(err.stack || err);
+				if(err.details) context.options.error(err.details);
+			}
+		},
 		handleRequest: function(filename, processRequest, req) {
 			// in lazy mode, rebuild on bundle request
 			if(context.options.lazy && (!context.options.filename || context.options.filename.test(filename)))
diff --git a/test/CompilerCallbacks.test.js b/test/CompilerCallbacks.test.js
new file mode 100644
index 000000000..586caeede
--- /dev/null
+++ b/test/CompilerCallbacks.test.js
@@ -0,0 +1,40 @@
+var should = require("should");
+var middleware = require("../middleware");
+require("mocha-sinon");
+
+describe("CompilerCallbacks", function() {
+	var plugins = {};
+	var compiler = {
+		watch: function() {},
+		plugin: function(name, callback) {
+			plugins[name] = callback;
+		}
+	};
+	beforeEach(function() {
+		plugins = {};
+	});
+
+	it("watch error should be reported to console", function(done) {
+		var error = new Error("Oh noes!");
+		this.sinon.stub(compiler, "watch", function(opts, callback) {
+			callback(error);
+		});
+		this.sinon.stub(console, "error");
+		middleware(compiler);
+		should.strictEqual(console.error.callCount, 1);
+		should.strictEqual(console.error.calledWith(error.stack), true);
+		done();
+	});
+
+	it("options.error should be used on watch error", function(done) {
+		this.sinon.stub(compiler, "watch", function(opts, callback) {
+			callback(new Error("Oh noes!"));
+		});
+		middleware(compiler, {
+			error: function(err) {
+				err.should.startWith("Error: Oh noes!");
+				done();
+			}
+		});
+	});
+});
diff --git a/test/Lazy.test.js b/test/Lazy.test.js
index cee7a9e2e..7ce532219 100644
--- a/test/Lazy.test.js
+++ b/test/Lazy.test.js
@@ -1,5 +1,6 @@
 var should = require("should");
 var middleware = require("../middleware");
+require("mocha-sinon");
 
 var doneStats = {
 	hasErrors: function() {
@@ -30,6 +31,7 @@ describe("Lazy mode", function() {
 	describe("builds", function() {
 		var req = { method: "GET", url: "/bundle.js" };
 		beforeEach(function() {
+			this.sinon.stub(console, "error");
 			instance = middleware(compiler, { lazy: true, quiet: true });
 		});
 		it("should trigger build", function(done) {
@@ -54,11 +56,13 @@ describe("Lazy mode", function() {
 			});
 		});
 
-		it("should pass through compiler error", function() {
-			compiler.run.callsArgWith(0, new Error("MyCompilerError"));
-			should.throws(function() {
-				instance(req, res, next);
-			}, "MyCompilerError");
+		it("should pass through compiler error", function(done) {
+			var error = new Error("MyCompilerError");
+			compiler.run.callsArgWith(0, error);
+			instance(req, res, next);
+			should.strictEqual(console.error.callCount, 1);
+			should.strictEqual(console.error.calledWith(error.stack), true);
+			done();
 		});
 	});