From dcc1fb2f83a3950285899fdee33d6bfff617f82f Mon Sep 17 00:00:00 2001 From: Steve Vitali Date: Fri, 19 Aug 2016 13:08:28 -0700 Subject: [PATCH 1/4] Improve error logging --- .../react-server/core/ClientController.js | 21 +++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index 9229a83d6..fd51ca6a3 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 ${componentType} render ${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. From b4fc465c27633c306f17d6ead2e62e033b894cbe Mon Sep 17 00:00:00 2001 From: Steve Vitali Date: Fri, 19 Aug 2016 13:09:01 -0700 Subject: [PATCH 2/4] Add error examples to error logging test page --- .../pages/error/logs.js | 132 +++++++++++++++++- 1 file changed, 125 insertions(+), 7 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..69e92d787 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 ( +

+ + +

+ ); + } +}; + +const ComponentWillReceivePropsError = ReceiveProps( + class extends Component { + componentWillReceiveProps(nextProps) { + throw Error('Error in componentWillReceiveProps'); + } + render() { + return componentWillReceiveProps error; + } + } +); + +const ComponentDidReceivePropsError = ReceiveProps( + class extends Component { + componentDidReceiveProps(nextProps) { + throw Error('Error in componentDidReceiveProps'); + } + render() { + return componentDidReceiveProps error; + } + } +); + +const ComponentWillUpdateError = ReceiveProps( + class extends Component { + componentWillUpdate(nextProps, nextState) { + throw Error('Error in componentWillUpdate'); + } + render() { + return componentWillUpdate error; + } + } +); + +const ComponentDidUpdateError = ReceiveProps( + class extends Component { + componentDidUpdate(prevProps, prevState) { + throw Error('Error in componentDidUpdate'); + } + render() { + return componentDidUpdate error; + } + } +); + +class ComponentDidMountError extends Component { + componentDidMount() { + throw Error('Error in componentDidMount'); + } + render() { + return

componentDidMount Error

; + } +} + +class ComponentWillMountError extends Component { + componentWillMount() { + throw Error('Error in componentWillMount'); + } + render() { + return

componentWillMount Error

; + } +} function fail() { - return Q().then(() => { throw new Error("Fail!") }); + return Q().then(() => { throw new Error('Fail'); }); +} + +const RootElementWhenPromiseFailure = (props => + +

RootElement when=failed promise

+ +); + +class MissingKeyPropInArrayIterator extends Component { + render() { + return ( +
+

Missing Key Prop in Array Iterator Warning

+ { [1,2,3,4].map(n => {n}) } +
+ ); + } } -export default class ErrorLogsPage { +export default class ErrorReportingPage { getElements() { return [ -
Check the logs
, - Nope, - ] +

Error Logging Tests

, +

Render error

, + , + , + , + , + , + , + , + , + , + ]; } } From 67eb7e2f7e386116cfa76e135ccef6fbc6dda879 Mon Sep 17 00:00:00 2001 From: Lida Wang Date: Thu, 25 Aug 2016 10:57:56 -0700 Subject: [PATCH 3/4] Fix linter issues --- packages/react-server-test-pages/pages/error/logs.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-server-test-pages/pages/error/logs.js b/packages/react-server-test-pages/pages/error/logs.js index 69e92d787..3ab90a527 100644 --- a/packages/react-server-test-pages/pages/error/logs.js +++ b/packages/react-server-test-pages/pages/error/logs.js @@ -34,7 +34,7 @@ const ReceiveProps = OriginalComponent => class extends Component { const ComponentWillReceivePropsError = ReceiveProps( class extends Component { - componentWillReceiveProps(nextProps) { + componentWillReceiveProps() { throw Error('Error in componentWillReceiveProps'); } render() { @@ -45,7 +45,7 @@ const ComponentWillReceivePropsError = ReceiveProps( const ComponentDidReceivePropsError = ReceiveProps( class extends Component { - componentDidReceiveProps(nextProps) { + componentDidReceiveProps() { throw Error('Error in componentDidReceiveProps'); } render() { @@ -56,7 +56,7 @@ const ComponentDidReceivePropsError = ReceiveProps( const ComponentWillUpdateError = ReceiveProps( class extends Component { - componentWillUpdate(nextProps, nextState) { + componentWillUpdate() { throw Error('Error in componentWillUpdate'); } render() { @@ -67,7 +67,7 @@ const ComponentWillUpdateError = ReceiveProps( const ComponentDidUpdateError = ReceiveProps( class extends Component { - componentDidUpdate(prevProps, prevState) { + componentDidUpdate() { throw Error('Error in componentDidUpdate'); } render() { @@ -98,7 +98,7 @@ function fail() { return Q().then(() => { throw new Error('Fail'); }); } -const RootElementWhenPromiseFailure = (props => +const RootElementWhenPromiseFailure = (() =>

RootElement when=failed promise

 From ea3710971f42233c49616a9ba18cf2f7dd0ca136 Mon Sep 17 00:00:00 2001 From: Lida Wang Date: Mon, 29 Aug 2016 13:33:49 -0700 Subject: [PATCH 4/4] Tweak the error logging statements to be more accurate --- packages/react-server/core/ClientController.js | 2 +- packages/react-server/core/renderMiddleware.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/react-server/core/ClientController.js b/packages/react-server/core/ClientController.js index fd51ca6a3..496ee8a67 100644 --- a/packages/react-server/core/ClientController.js +++ b/packages/react-server/core/ClientController.js @@ -685,7 +685,7 @@ class ClientController extends EventEmitter { const componentType = typeof element.type === 'function' ? element.props.children.type.name : 'element'; - logger.error(`Error with ${componentType} render ${index}`, e); + logger.error(`Error with element ${componentType}'s lifecycle methods at index ${index}`, e); }) ).catch(e => logger.error(`Error with element promise ${index}`, e)) ), diff --git a/packages/react-server/core/renderMiddleware.js b/packages/react-server/core/renderMiddleware.js index 58b64ae6b..56c1ca54e 100644 --- a/packages/react-server/core/renderMiddleware.js +++ b/packages/react-server/core/renderMiddleware.js @@ -800,7 +800,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