Fix timeout error handling (#716)

* Fix timeout error handling

The main issues addressed here are:

1. When renderMiddleware timesout, it will send down what ever it has
left but it fails to close out the body and html.

2. In ClientController there is a race condition between nodeArrival and
failArrival. nodeArrival will receive a bunch of nodes since before
failArrival is sent the nodes we have left are sent down. If failArrival
runs an resolves `retVal` before the nodes can actually render the
`_previouslyRender` flag will be set. Once that is set and the elements
try to render, because `_previouslyRender` is set at the point
`_cleanup` will be called which will wipe away everything on the page.

* handle element render failures without failing writeBody lifecycle method

* Fix react-server-intergration test failures

packages/react-server/core/ClientController.js

@@ -529,6 +529,17 @@ class ClientController extends EventEmitter {
 		// These resolve with React elements when their data
 		// dependencies are fulfilled.
 		var elementPromises = PageUtil.standardizeElements(page.getElements());
+		var timeoutDfd = [];
+		var elementPromisesOr = elementPromises.map((promise, index) => {
+			var orPromise = Q.defer();
+			timeoutDfd[index] = Q.defer();
+
+			promise.then(orPromise.resolve);
+			promise.catch(orPromise.reject);
+			timeoutDfd[index].promise.catch(orPromise.reject);
+
+			return orPromise.promise;
+		});
 
 		// These resolve with DOM mount points for the elements.
 		//
@@ -672,7 +683,7 @@ class ClientController extends EventEmitter {
 		// Always render in order to proritize content higher in the
 		// page.
 		//
-		elementPromises.reduce((chain, promise, index) => chain
+		elementPromisesOr.reduce((chain, promise, index) => chain
 			.then(() => promise
 				.then(element => rootNodePromises[index]
 					.then(root => renderElement(element, root, index))
@@ -691,7 +702,14 @@ class ClientController extends EventEmitter {
 		// Look out for a failsafe timeout from the server on our
 		// first render.
 		if (!this._previouslyRendered){
-			this._failDfd.promise.then(retval.resolve);
+			this._failDfd.promise.then(() => {
+				elementPromises.forEach((promise, index) => {
+					//Reject any elements that have failed to render
+					if (promise.isPending()) {
+						timeoutDfd[index].reject(`Error with element ${index}, it failed to render within timeout time`);
+					}
+				});
+			});
 		}
 
 		return retval.promise.then(() => {

packages/react-server/core/renderMiddleware.js

@@ -727,10 +727,11 @@ function writeBody(req, res, context, start, page) {
 	,   timeRemaining = totalWait - (new Date - start)
 
 	var retval = Q.defer();
+	var writeBodyDfd = Q.defer();
 
 	// If we exceed the timeout then we'll just send empty elements for
 	// anything that hadn't rendered yet.
-	retval.promise.catch(() => {
+	writeBodyDfd.promise.catch((err) => {
 
 		// Write out what we've got.
 		writeElements(res, rendered.map(
@@ -742,13 +743,17 @@ function writeBody(req, res, context, start, page) {
 
 		// Let the client know it's not getting any more data.
 		renderScriptsAsync([{ text: `__reactServerClientController.failArrival()` }], res)
+
+		//Log timeout error but still resolve so we continue in the lifecycle process
+		logger.error("Error in writeBody", err);
+		retval.resolve();
 	});
 
-	Q.all(dfds.map(dfd => dfd.promise)).then(retval.resolve);
+	Q.all(dfds.map(dfd => dfd.promise)).then(writeBodyDfd.resolve);
 
 	const timeout = setTimeout(() => {
 		// give some additional information when we time out
-		retval.reject({
+		writeBodyDfd.reject({
 			message: "Timed out rendering.",
 			// `timeRemaining` is how long we waited before timing out
 			timeWaited: timeRemaining,
@@ -765,7 +770,11 @@ function writeBody(req, res, context, start, page) {
 	}, timeRemaining);
 
 	// Don't leave dead timers hanging around.
-	retval.promise.then(() => clearTimeout(timeout));
+	writeBodyDfd.promise.then(() => {
+		clearTimeout(timeout);
+		//writeBody ran successfully, sweet
+		retval.resolve();
+	});
 
 	return retval.promise;
 }