Skip to content

Commit

Permalink
Improved error view handling, returning missing component in dev, cus…
Browse files Browse the repository at this point in the history
…tom error always if declared
  • Loading branch information
thomasdigby committed Jul 9, 2017
1 parent f3179cc commit b771834
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 46 deletions.
1 change: 1 addition & 0 deletions src/server/handle-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ export default ({ server, config, assets }) => {
// No HTML found for this path, or cache expired
// Regenerate HTML from scratch
const html = renderHtml({
response,
renderProps,
loadContext,
asyncProps,
Expand Down
2 changes: 2 additions & 0 deletions src/server/render/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import DefaultHTML from './default-html'
import RenderError from '../../shared/render-error'

export default ({
response,
renderProps = false,
loadContext,
asyncProps,
Expand All @@ -25,6 +26,7 @@ export default ({
loadContext={loadContext}
/> :
<RenderError
response={response}
config={loadContext}
/>
)
Expand Down
24 changes: 12 additions & 12 deletions src/shared/default-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'

const DefaultError = ({
message,
status,
code,
children
}) =>
<section style={{
Expand All @@ -19,16 +19,16 @@ const DefaultError = ({
<path style={{ fill: '#E5E5E5' }} d="M7.6 20.67h1.93v18.3H7.4c-.9.04-1.75.3-1.75.3V21.6s.86-.95 1.94-.95zm5.46-11.3h3.88V27.7h-3.88V9.38zm14.82.03h3.88v18.3h-3.88V9.4zm14.83 0h3.9v18.3h-3.9V9.4zm7.5 11.3h2.4c.8 0 1.5-.22 1.5-.22v18s-.4.44-1.73.5H50.2v-18.3zM13.1 31.94H17v18.3h-3.9v-18.3zm14.8.06h3.88v18.2H27.9V32zm14.83-.04h3.9v18.3h-3.9v-18.3zm-22.2-11.3h3.9v18.3h-3.9v-18.3zm14.83 0h3.88v18.3h-3.9v-18.3zM7.46 43.24H9.5V57.6c0 1.07-.87 1.95-1.94 1.95-1.08 0-1.95-.88-1.95-1.96V43.9s.5-.62 1.8-.73zM52.2 16.36h-2.1V2.14c0-1.08.87-1.96 1.94-1.96 1.07 0 1.94.88 1.94 1.96V15.9s-.72.47-1.8.47z"/>
</svg>
{
status &&
<p style={{
color: '#c0c0c0',
fontSize: '28px',
fontWeight: 'bold',
marginBottom: '10px',
marginTop: '20px'
}}>
{status}
</p>
code &&
<p style={{
color: '#c0c0c0',
fontSize: '28px',
fontWeight: 'bold',
marginBottom: '10px',
marginTop: '20px'
}}>
{code}
</p>
}
<h1 style={{
color: '#c0c0c0',
Expand All @@ -46,7 +46,7 @@ DefaultError.defaultProps = {
}
DefaultError.propTypes = {
message: PropTypes.string,
status: PropTypes.string,
code: PropTypes.number,
children: PropTypes.node
}

Expand Down
7 changes: 5 additions & 2 deletions src/shared/fetch-data.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React, { Component } from 'react'
import PropTypes from 'prop-types'
import AsyncProps from 'async-props'
import idx from 'idx'
import { generate as uid } from 'shortid'
import fetchRouteData from './fetch-route-data'
import RenderError from './render-error'
Expand Down Expand Up @@ -40,11 +41,13 @@ const fetchData = (TopLevelComponent, route) => {

render() {
const response = handleApiResponse(this.props.data, this.props.route)

// check data/component exists and isn't a server errored response
if (!TopLevelComponent || !response) {
if (!TopLevelComponent || idx(response, _ => _.code)) {
return (
<RenderError
data={response}
response={response}
missing={!TopLevelComponent}
config={this.props.route.config}
/>
)
Expand Down
16 changes: 8 additions & 8 deletions src/shared/missing-view.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,18 @@ import PropTypes from 'prop-types'
import ObjectInspector from 'react-object-inspector'
import DefaultError from './default-error'

const MissingView = (data) =>
<DefaultError message="Missing Component">
const MissingView = (response) =>
<DefaultError {...response}>
{
data &&
<ObjectInspector
data={data}
initialExpandedPaths={['root']}
/>
response &&
<ObjectInspector
data={response}
initialExpandedPaths={['root']}
/>
}
</DefaultError>

MissingView.propTypes = {
data: PropTypes.object
response: PropTypes.object
}
export default MissingView
20 changes: 12 additions & 8 deletions src/shared/render-error.js
Original file line number Diff line number Diff line change
@@ -1,25 +1,29 @@
import React from 'react'
import idx from 'idx'
import PropTypes from 'prop-types'
import has from 'lodash/has'
import DefaultError from './default-error'
import MissingView from './missing-view'

const RenderError = ({ config, data }) => {
const RenderError = ({ config, response, missing }) => {
// render custom error or default if custom error not declared
let ErrorView = has(config, 'components.CustomError') ?
let ErrorView = idx(config, _ => _.components.CustomError) ?
config.components.CustomError :
DefaultError
// always render missing view in development
if (__DEV__) {
ErrorView = () => MissingView(data)
// render missing component only in DEV
if (__DEV__ && missing) {
ErrorView = MissingView
response = {
message: 'Missing component'
}
}
// return one of the error views
return <ErrorView />
return <ErrorView {...response} />
}

RenderError.propTypes = {
config: PropTypes.object,
data: PropTypes.any
missing: PropTypes.bool,
response: PropTypes.object
}

export default RenderError
10 changes: 9 additions & 1 deletion src/shared/route-wrapper.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react'
import { Route } from 'react-router'
import { generate as uid } from 'shortid'
import HTTPStatus from 'http-status'

import defaultRoutes from './default-routes'
import fetchData from './fetch-data'
Expand All @@ -16,8 +17,15 @@ const ComponentWrapper = (component, route) => {

const RouteWrapper = (config) => {
// if user routes have been defined, take those in preference to the defaults
const errorResponse = {
code: HTTPStatus.NOT_FOUND,
message: HTTPStatus[HTTPStatus.NOT_FOUND]
}
const routes = config.routes || defaultRoutes(config.components)
const ErrorComponent = () => RenderError({ config })
const ErrorComponent = () => RenderError({
config,
response: errorResponse
})
// loops over routes and return react-router <Route /> components
return (
<Route
Expand Down
68 changes: 53 additions & 15 deletions test/tests/component.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React from 'react'
import HTTPStatus from 'http-status'
import Helmet from 'react-helmet'
import { expect } from 'chai'
import request from 'request'
Expand Down Expand Up @@ -140,60 +141,97 @@ describe('Error view rendering', () => {
.reply(200, dataPosts.data)
.get('/wp-json/wp/v2/posts?slug=slug&_embed')
.reply(200, dataPage)
.get('/wp-json/wp/v2/pages?slug=404-route&_embed')
.times(5)
.reply(404, { data: { status: 404 } })
done()
})

afterEach(() => tapestry.server.stop())

it('CustomError missing in DEV, render Missing View', (done) => {
it('Show MissingView in DEV if component missing', (done) => {
tapestry = bootServer(config)
tapestry.server.on('start', () => {
request
.get(tapestry.server.info.uri, (err, res, body) => {
expect(body).to.contain('Missing Component')
expect(body).to.contain('Missing component')
done()
})
})
})

it('CustomError declared in DEV, render Missing View', (done) => {
it('Show DefaultError in PROD if component missing', (done) => {
tapestry = bootServer(config, { __DEV__: false })
tapestry.server.on('start', () => {
request
.get(tapestry.server.info.uri, (err, res, body) => {
expect(body).to.contain('Application Error')
done()
})
})
})

it('Show DefaultError if API 404', (done) => {
tapestry = bootServer({
...config,
components: {
CustomError: () => <p>Custom Error</p>
}
components: { Page: () => <p>Hello</p> }
})
tapestry.server.on('start', () => {
request
.get(`${tapestry.server.info.uri}/404-route`, (err, res, body) => {
expect(body).to.contain(404)
expect(body).to.contain(HTTPStatus[404])
done()
})
})
})

it('Show DefaultError if route 404', (done) => {
tapestry = bootServer({
...config,
components: { Page: () => <p>Hello</p> }
})
tapestry.server.on('start', () => {
request
.get(`${tapestry.server.info.uri}/route/not/matched/in/any/way`, (err, res, body) => {
expect(body).to.contain('Missing Component')
expect(body).to.contain(404)
expect(body).to.contain(HTTPStatus[404])
done()
})
})
})

it('CustomError missing in PROD, render Default Error', (done) => {
tapestry = bootServer(config, { __DEV__: false })
it('Show CustomError if defined', (done) => {
tapestry = bootServer({
...config,
components: {
CustomError: () => <p>Custom Error</p>,
Page: () => <p>Hello</p>
}
})
tapestry.server.on('start', () => {
request
.get(tapestry.server.info.uri, (err, res, body) => {
expect(body).to.contain('Application Error')
.get(`${tapestry.server.info.uri}/404-route`, (err, res, body) => {
expect(body).to.contain('Custom Error')
done()
})
})
})

it('CustomError declared in PROD, render CustomError', (done) => {
it('Status code and message are available to CustomError', (done) => {
tapestry = bootServer({
...config,
components: {
CustomError: () => <p>Custom Error</p>
CustomError: ({ code, message }) => <p>Custom Error {code} {message}</p>,
Page: () => <p>Hello</p>
}
}, { __DEV__: false })
})
tapestry.server.on('start', () => {
request
.get(`${tapestry.server.info.uri}/route/not/matched/in/any/way`, (err, res, body) => {
.get(`${tapestry.server.info.uri}/404-route`, (err, res, body) => {
expect(body).to.contain('Custom Error')
expect(body).to.contain('404')
expect(body).to.contain('Not Found')
done()
})
})
Expand Down

0 comments on commit b771834

Please sign in to comment.