Render server errors in the browser in dev mode

Currently we just display internal server error. Rendering the error in the browser should lead to a better developer UX. Refs #108
fusionjs · Jan 8, 2018 · 1dc1648 · 1dc1648
1 parent 600099d
commit 1dc1648
Show file tree

Hide file tree

Showing 12 changed files with 67 additions and 15 deletions.
diff --git a/build/dev-runtime.js b/build/dev-runtime.js
@@ -163,7 +163,8 @@ module.exports.DevelopmentRuntime = function({
             req.pipe(proxyReq).pipe(res);
           },
           error => {
-            renderError(res, error);
+            res.write(renderError(error));
+            res.end();
           }
         );
       });

diff --git a/build/server-error.js b/build/server-error.js
@@ -3,19 +3,19 @@ const React = require('react');
 const RedBox = require('redbox-react').RedBoxError;
 const ReactDOMServer = require('react-dom/server');
 
-function renderError(res, error) {
-  res.write(
-    '<!DOCTYPE html><html><head><title>Server error</title></head><body>'
-  );
+function renderError(error) {
+  const content = [
+    '<!DOCTYPE html><html><head><title>Server error</title></head><body>',
+  ];
 
   const displayError = typeof error === 'string' ? new Error(error) : error;
   const errorComponent = ReactDOMServer.renderToString(
     React.createElement(RedBox, {error: displayError})
   );
-  res.write(errorComponent);
+  content.push(errorComponent);
 
-  res.write('</body></html>');
-  res.end();
+  content.push('</body></html>');
+  return content.join('');
 }
 
 module.exports.renderError = renderError;
diff --git a/entries/server-entry.js b/entries/server-entry.js
@@ -5,10 +5,12 @@ import main from '__FRAMEWORK_SHARED_ENTRY__';
 import CompilationMetaDataFactory from '../plugins/compilation-metadata-plugin';
 import AssetsFactory from '../plugins/assets-plugin';
 import ContextFactory from '../plugins/context-plugin';
+import ServerErrorFactory from '../plugins/server-error-plugin';
 
 const CompilationMetaData = CompilationMetaDataFactory();
 const Assets = AssetsFactory();
 const Context = ContextFactory();
+const ServerErrorHandling = ServerErrorFactory();
 
 /*
 Webpack has a configuration option called `publicPath`, which determines the
@@ -59,6 +61,9 @@ export async function start({port}) {
 async function reload() {
   const app = await initialize();
   app.plugins = [Assets, Context].concat(app.plugins);
+  if (__DEV__) {
+    app.plugins.unshift(ServerErrorHandling);
+  }
   state.serve = app.callback();
   state.app = app;
 }

diff --git a/package.json b/package.json
@@ -36,6 +36,7 @@
     "compose-middleware": "4.0.0",
     "compression-webpack-plugin": "^1.1.3",
     "core-js": "^2.5.3",
+    "debug": "^3.1.0",
     "enzyme-adapter-react-16": "^1.1.1",
     "enzyme-to-json": "^3.3.0",
     "es6-object-assign": "^1.1.0",

diff --git a/plugins/server-error-plugin.js b/plugins/server-error-plugin.js
@@ -0,0 +1,14 @@
+/* eslint-env node */
+
+const renderError = require('../build/server-error').renderError;
+
+module.exports = function() {
+  return async function middleware(ctx, next) {
+    try {
+      await next();
+    } catch (err) {
+      ctx.status = err.statusCode || err.status || 500;
+      ctx.body = renderError(err);
+    }
+  };
+};
diff --git a/test/cli/dev.js b/test/cli/dev.js
@@ -65,12 +65,17 @@ test('`fusion dev` works with assets with cdnUrl', async t => {
 });
 
 test('`fusion dev` top-level error', async t => {
-  const dir = path.resolve(
-    __dirname,
-    '../fixtures/server-error-route-component'
-  );
+  const dir = path.resolve(__dirname, '../fixtures/server-startup-error');
+  const {res, proc} = await dev(`--dir=${dir}`);
+  t.ok(res.includes('server-startup-error'));
+  proc.kill();
+  t.end();
+});
+
+test('`fusion dev` server render error', async t => {
+  const dir = path.resolve(__dirname, '../fixtures/server-render-error');
   const {res, proc} = await dev(`--dir=${dir}`);
-  t.ok(res.includes('top-level-route-error'));
+  t.ok(res.includes('server-render-error'));
   proc.kill();
   t.end();
 });
diff --git a/test/fixtures/server-error-route-component/src/home.js b/test/fixtures/server-error-route-component/src/home.js
diff --git a/test/fixtures/server-render-error/src/home.js b/test/fixtures/server-render-error/src/home.js
@@ -0,0 +1,7 @@
+import React from 'react';
+
+const Home = () => {
+  throw new Error('server-render-error');
+  return null;
+};
+export default Home;
diff --git a/test/fixtures/server-render-error/src/main.js b/test/fixtures/server-render-error/src/main.js
@@ -0,0 +1,13 @@
+import React from 'react';
+import App from 'fusion-react';
+import Router, {Route, Switch} from 'fusion-plugin-react-router';
+
+import Home from './home.js';
+
+export default () => {
+  const app = new App(<Switch>
+    <Route exact path="/" component={Home} />
+  </Switch>);
+  app.plugin(Router, {});
+  return app;
+};
diff --git a/test/fixtures/server-startup-error/src/home.js b/test/fixtures/server-startup-error/src/home.js
@@ -0,0 +1 @@
+throw new Error('server-startup-error');
diff --git a/.../server-error-route-component/src/main.js → ...fixtures/server-startup-error/src/main.js b/.../server-error-route-component/src/main.js → ...fixtures/server-startup-error/src/main.js
diff --git a/test/run-command.js b/test/run-command.js
@@ -75,7 +75,13 @@ async function waitForServer(port) {
       });
       started = true;
     } catch (e) {
-      numTries++;
+      // Allow returning true for 500 status code errors to test error states
+      if (e.statusCode === 500) {
+        started = true;
+        res = e.response.body;
+      } else {
+        numTries++;
+      }
     }
   }
   if (!started) {