From 73a5fe479ca7c5a5432f039718c7d0be77042ce4 Mon Sep 17 00:00:00 2001
From: Lida Wang <jintuishiju@gmail.com>
Date: Mon, 29 Aug 2016 13:50:50 -0700
Subject: [PATCH] Rebased sjv improve error logging (#626)

* Improve error logging

* Add error examples to error logging test page

* Fix linter issues

* Tweak the error logging statements to be more accurate
---
 .../pages/error/logs.js                       | 132 +++++++++++++++++-
 .../react-server/core/ClientController.js     |  21 ++-
 .../react-server/core/renderMiddleware.js     |   2 +-
 3 files changed, 141 insertions(+), 14 deletions(-)

diff --git a/packages/react-server-test-pages/pages/error/logs.js b/packages/react-server-test-pages/pages/error/logs.js
index 2cb818717..3ab90a527 100644
--- a/packages/react-server-test-pages/pages/error/logs.js
+++ b/packages/react-server-test-pages/pages/error/logs.js
@@ -1,16 +1,134 @@
-import Q from "q";
+import Q from 'q';
+import { Component } from 'react';
+import { RootElement } from 'react-server';
 
-import {RootElement} from "react-server";
+class RenderError extends Component {
+	render() {
+		throw new Error('Error in render');
+	}
+}
+
+const ReceiveProps = OriginalComponent => class extends Component {
+	constructor(props) {
+		super(props);
+		this.state = {
+			randomProp: Math.random(),
+		};
+	}
+	updateState() {
+		this.setState({
+			randomProp: Math.random(),
+		});
+	}
+	render() {
+		return (
+			<h2>
+				<button onClick={this.updateState.bind(this)}>
+					Click to update props
+				</button>
+				<OriginalComponent {...this.state}/>
+			</h2>
+		);
+	}
+};
+
+const ComponentWillReceivePropsError = ReceiveProps(
+	class extends Component {
+		componentWillReceiveProps() {
+			throw Error('Error in componentWillReceiveProps');
+		}
+		render() {
+			return <span>componentWillReceiveProps error</span>;
+		}
+	}
+);
+
+const ComponentDidReceivePropsError = ReceiveProps(
+	class extends Component {
+		componentDidReceiveProps() {
+			throw Error('Error in componentDidReceiveProps');
+		}
+		render() {
+			return <span>componentDidReceiveProps error</span>;
+		}
+	}
+);
+
+const ComponentWillUpdateError = ReceiveProps(
+	class extends Component {
+		componentWillUpdate() {
+			throw Error('Error in componentWillUpdate');
+		}
+		render() {
+			return <span>componentWillUpdate error</span>;
+		}
+	}
+);
+
+const ComponentDidUpdateError = ReceiveProps(
+	class extends Component {
+		componentDidUpdate() {
+			throw Error('Error in componentDidUpdate');
+		}
+		render() {
+			return <span>componentDidUpdate error</span>;
+		}
+	}
+);
+
+class ComponentDidMountError extends Component {
+	componentDidMount() {
+		throw Error('Error in componentDidMount');
+	}
+	render() {
+		return <h2>componentDidMount Error</h2>;
+	}
+}
+
+class ComponentWillMountError extends Component {
+	componentWillMount() {
+		throw Error('Error in componentWillMount');
+	}
+	render() {
+		return <h2>componentWillMount Error</h2>;
+	}
+}
 
 function fail() {
-	return Q().then(() => { throw new Error("Fail!") });
+	return Q().then(() => { throw new Error('Fail'); });
+}
+
+const RootElementWhenPromiseFailure = (() =>
+	<RootElement when={fail()}>
+		<h2>RootElement when=failed promise</h2>
+	</RootElement>
+);
+
+class MissingKeyPropInArrayIterator extends Component {
+	render() {
+		return (
+			<div>
+				<h2>Missing Key Prop in Array Iterator Warning</h2>
+				{ [1,2,3,4].map(n => <span>{n}</span>) }
+			</div>
+		);
+	}
 }
 
-export default class ErrorLogsPage {
+export default class ErrorReportingPage {
 	getElements() {
 		return [
-			<div>Check the logs</div>,
-			<RootElement when={fail()}>Nope</RootElement>,
-		]
+			<h1>Error Logging Tests</h1>,
+			<h2>Render error</h2>,
+			<RenderError/>,
+			<ComponentWillMountError/>,
+			<ComponentDidMountError/>,
+			<ComponentWillReceivePropsError/>,
+			<ComponentDidReceivePropsError/>,
+			<ComponentWillUpdateError/>,
+			<ComponentDidUpdateError/>,
+			<RootElementWhenPromiseFailure/>,
+			<MissingKeyPropInArrayIterator/>,
+		];
 	}
 }
diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js
index 9229a83d6..496ee8a67 100644
--- a/packages/react-server/core/ClientController.js
+++ b/packages/react-server/core/ClientController.js
@@ -675,12 +675,21 @@ class ClientController extends EventEmitter {
 		// Always render in order to proritize content higher in the
 		// page.
 		//
-		elementPromises.reduce((chain, promise, index) => chain.then(
-			() => promise.then(element => rootNodePromises[index]
-				.then(root => renderElement(element, root, index))
-				.catch(e => logger.error(`Error with element render ${index}`, e))
-			).catch(e => logger.error(`Error with element promise ${index}`, e))
-		), Q()).then(retval.resolve);
+		elementPromises.reduce((chain, promise, index) => chain
+			.then(() => promise
+				.then(element => rootNodePromises[index]
+					.then(root => renderElement(element, root, index))
+					.catch(e => {
+						// The only case where this should evaluate to false is
+						// when `element` is a containerClose/containerOpen object
+						const componentType = typeof element.type === 'function'
+							? element.props.children.type.name
+							: 'element';
+						logger.error(`Error with element ${componentType}'s lifecycle methods at index ${index}`, e);
+					})
+				).catch(e => logger.error(`Error with element promise ${index}`, e))
+			),
+		Q()).then(retval.resolve);
 
 		// Look out for a failsafe timeout from the server on our
 		// first render.
diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js
index 91a9e633f..348fc880d 100644
--- a/packages/react-server/core/renderMiddleware.js
+++ b/packages/react-server/core/renderMiddleware.js
@@ -804,7 +804,7 @@ function renderElement(res, element, context) {
 		// the `data-react-server-root-id` div for this component.  We need
 		// to close it out and move on.  This is a bummer, and we'll
 		// log it, but it's too late to totally bail out.
-		logger.error(`Error rendering element ${name}`, err);
+		logger.error(`Error with element ${name}'s lifecycle methods`, err);
 	}
 
 	// We time how long _this_ element's render took, and also how long