500 Series Error Handling & Stack Traces

Fixes #825

- Changes the way the error middleware is delivered in server.js, moving
  all the logic back into errorHandling.js
- Alters error logging to use console.error (probably more appropriate) instead
  of console.log
- Changes error tests to accomodate for these alterations
- Alters user-error and error hbs templates to incorporate stack traces
- Adds additional styling for error pages to accomodate stack traces
- Added logic to parse and deliver formatted stack traces

Notes:
======

- Jslint gets in the way of the regex I've got to use to parse the stack.
  (It cites 'security reasons' which are not relevant in this case.)
  I needed to add a condition to relax it at the top of errorHandling.js
- The stack trace should probably be added as a partial, but I figured it
  was out of scope for this PR.

core/client/assets/sass/layouts/errors.scss

@@ -77,10 +77,31 @@
     }
 }
 
-/* =============================================================================
+.error-stack {
-   404
+    margin: 1em auto;
-   ============================================================================= */
+    padding: 2em;
-
+    max-width: 800px;
-.error-404 {
+    background-color: rgba(255,255,255,0.3);
-    width: 300px;
+}
+
+.error-stack-list {
+    list-style-type: none;
+    padding: 0;
+    margin: 0;
+}
+
+.error-stack-list li {
+    display: block;
+    
+    &::before {
+        color: #BBB;
+        content: "↪︎";
+        display: inline-block;
+        font-size: 1.2em;
+        margin-right: 0.5em;
+    }
+}
+
+.error-stack-function {
+    font-weight: bold;
 }

core/server.js

@@ -283,24 +283,8 @@ when.all([ghost.init(), helpers.loadCoreHelpers(ghost)]).then(function () {
     // 404 Handler
     server.use(errors.render404Page);
 
-    // TODO: Handle all application errors (~500)
+    // 500 Handler
-    // Just stubbed at this stage!
+    server.use(errors.render500Page);
-    server.use(function error500Handler(err, req, res, next) {
-        if (!err || !(err instanceof Error)) {
-            next();
-        }
-
-        // For the time being, just log and continue.
-        errors.logError(err, "Middleware", "Ghost caught a processing error in the middleware layer.");
-        next(err);
-    });
-
-    // All other errors
-    if (server.get('env') === "production") {
-        server.use(express.errorHandler({ dumpExceptions: false, showStack: false }));
-    } else {
-        server.use(express.errorHandler({ dumpExceptions: true, showStack: true }));
-    }
 
     // ## Routing
 

core/server/errorHandling.js

@@ -1,3 +1,4 @@
+/*jslint regexp: true */
 var _ = require('underscore'),
     colors = require("colors"),
     fs = require('fs'),
@@ -29,23 +30,30 @@ errors = {
     },
 
     logError: function (err, context, help) {
+        var stack = err ? err.stack : null;
         err = err.message || err || "Unknown";
         // TODO: Logging framework hookup
         // Eventually we'll have better logging which will know about envs
-        if (process.env.NODE_ENV === 'development' || process.env.NODE_ENV === 'staging'
+        if ((process.env.NODE_ENV === 'development' ||
-                || process.env.NODE_ENV === 'production') {
+            process.env.NODE_ENV === 'staging' ||
+            process.env.NODE_ENV === 'production')) {
 
-            console.log("\nERROR:".red, err.red, err.stack || "");
+            console.error("\nERROR:".red, err.red);
 
             if (context) {
-                console.log(context);
+                console.error(context);
             }
 
             if (help) {
-                console.log(help.green);
+                console.error(help.green);
             }
+
             // add a new line
-            console.log("");
+            console.error("");
+
+            if (stack) {
+                console.error(stack, "\n");
+            }
         }
     },
 
@@ -74,17 +82,54 @@ errors = {
     },
 
     renderErrorPage: function (code, err, req, res, next) {
+
+        function parseStack(stack) {
+            if (typeof stack !== "string") {
+                return stack;
+            }
+
+            // TODO: split out line numbers
+            var stackRegex = /\s*at\s*(\w+)?\s*\(([^\)]+)\)\s*/i;
+
+            return (
+                stack
+                    .split(/[\r\n]+/)
+                    .slice(1)
+                    .map(function (line) {
+                        var parts = line.match(stackRegex);
+                        if (!parts) {
+                            return null;
+                        }
+
+                        return {
+                            "function": parts[1],
+                            "at": parts[2]
+                        };
+                    })
+                    .filter(function (line) {
+                        return !!line;
+                    })
+            );
+        }
+
         // Render the error!
         function renderErrorInt(errorView) {
+            var stack = null;
+
+            if (process.env.NODE_ENV !== "production" && err.stack) {
+                stack = parseStack(err.stack);
+            }
+
             // TODO: Attach node-polyglot
             res.render((errorView || "error"), {
                 message: err.message || err,
-                code: code
+                code: code,
+                stack: stack
             });
         }
 
         if (code >= 500) {
-            this.logError(err, "ErrorPage");
+            this.logError(err, "ErrorPage", "Ghost caught a processing error in the middleware layer.");
         }
 
         // Are we admin? If so, don't worry about the user template
@@ -119,6 +164,13 @@ errors = {
     render404Page: function (req, res, next) {
         var message = res.isAdmin ? "No Ghost Found" : "Page Not Found";
         this.renderErrorPage(404, message, req, res, next);
+    },
+
+    render500Page: function (err, req, res, next) {
+        if (!err || !(err instanceof Error)) {
+            next();
+        }
+        errors.renderErrorPage(500, err, req, res, next);
     }
 };
 
@@ -130,7 +182,8 @@ _.bindAll(
     "logAndThrowError",
     "logErrorWithRedirect",
     "renderErrorPage",
-    "render404Page"
+    "render404Page",
+    "render500Page"
 );
 
 module.exports = errors;

core/server/views/error.hbs

@@ -10,3 +10,18 @@
         </section>
     </section>
 </section>
+{{#if stack}}
+    <section class="error-stack">
+        <h3>Stack Trace</h3>
+        <p><strong>{{message}}</strong></p>
+        <ul class="error-stack-list">
+            {{#foreach stack}}
+                <li>
+                    at
+                    {{#if function}}<em class="error-stack-function">{{function}}</em>{{/if}}
+                    <span class="error-stack-file">({{at}})</span>
+                </li>
+            {{/foreach}}
+        </ul>
+    </section>
+{{/if}}

core/server/views/user-error.hbs

@@ -25,7 +25,10 @@
 			<section class="error-content error-404 js-error-container">
 				<section class="error-details">
 					<figure class="error-image">
-					  <img class="error-ghost" src="/ghost/img/404-ghost@2x.png" srcset="/ghost/img/404-ghost.png 1x, /ghost/img/404-ghost@2x.png 2x"/>
+						<img
+							class="error-ghost"
+							src="/ghost/img/404-ghost@2x.png"
+							srcset="/ghost/img/404-ghost.png 1x, /ghost/img/404-ghost@2x.png 2x"/>
 					</figure>
 					<section class="error-message">
 						<h1 class="error-code">{{code}}</h1>
@@ -33,6 +36,21 @@
 					</section>
 				</section>
 			</section>
+			{{#if stack}}
+				<section class="error-stack">
+					<h3>Stack Trace</h3>
+					<p><strong>{{message}}</strong></p>
+					<ul class="error-stack-list">
+						{{#foreach stack}}
+							<li>
+								at
+								{{#if function}}<em class="error-stack-function">{{function}}</em>{{/if}}
+								<span class="error-stack-file">({{at}})</span>
+							</li>
+						{{/foreach}}
+					</ul>
+				</section>
+			{{/if}}
 		</main>
 	</body>
 </html>

core/test/unit/errorHandling_spec.js

@@ -43,7 +43,7 @@ describe("Error handling", function () {
 
     it("logs errors", function () {
         var err = new Error("test1"),
-            logStub = sinon.stub(console, "log");
+            logStub = sinon.stub(console, "error");
 
         // give environment a value that will console log
         process.env.NODE_ENV = "development";
@@ -75,7 +75,7 @@ describe("Error handling", function () {
                     return;
                 }
             },
-            logStub = sinon.stub(console, "log"),
+            logStub = sinon.stub(console, "error"),
             redirectStub = sinon.stub(res, "redirect");
 
         // give environment a value that will console log