Error handler with context

CHANGELOG.md

@@ -5,6 +5,21 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/)
 and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html).
 
+## [Unreleased]
+
+- The router no longer mutate errors to avoid issues with non-extensible objects.
+  (BREAKING CHANGE [#158](https://github.com/kriasoft/universal-router/pull/158)).
+
+**Migration from v6 to v7:**
+- If your code relies on `error.context` or `error.code` you still can access them
+  using `errorHandler` option:
+  ```js
+  errorHandler(error, context) {
+    const code = error.status || 500
+    console.log(error, context, code)
+  }
+  ```
+
 ## [6.0.0] - 2018-02-06
 
 - No special configuration is required for your bundler anymore (say hi to [parcel.js](https://parceljs.org/)).

docs/api.md

@@ -15,11 +15,9 @@ Second `options` argument is optional where you can pass the following:
 * `resolveRoute` - function for any custom route handling logic.\
   For example you can define this option to work with routes in declarative manner.\
   By default the router calls the `action` method of matched route.
-* `errorHandler` - function for global error handling. Called with a single
-  [Error](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error) 
-  argument every time the route is not found or threw an error which always contain
-  [http status `code`](https://en.wikipedia.org/wiki/List_of_HTTP_status_codes) and
-  [current `context`](#context) for your convenience.
+* `errorHandler` - function for global error handling. Called with an
+  [Error](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error)
+  and [Context](#context) arguments every time the route is not found or threw an error.
 
 ```js
 import UniversalRouter from 'universal-router'
@@ -47,10 +45,10 @@ const options = {
     }
     return undefined
   },
-  errorHandler(error) {
+  errorHandler(error, context) {
     console.error(error)
-    console.dir(error.context)
-    return error.code === 404
+    console.info(context)
+    return error.status === 404
       ? '<h1>Page Not Found</h1>'
       : '<h1>Oops! Something went wrong</h1>'
   }

src/UniversalRouter.js

@@ -74,9 +74,8 @@ class UniversalRouter {
       }
 
       if (matches.done) {
-        const error = new Error('Page not found')
-        error.context = context
-        error.code = 404
+        const error = new Error('Route not found')
+        error.status = 404
         return Promise.reject(error)
       }
 
@@ -95,10 +94,8 @@ class UniversalRouter {
     return Promise.resolve()
       .then(() => next(true, this.root))
       .catch((error) => {
-        error.context = error.context || currentContext
-        error.code = error.code || 500
         if (this.errorHandler) {
-          return this.errorHandler(error)
+          return this.errorHandler(error, currentContext)
         }
         throw error
       })

test/UniversalRouter.test.js

@@ -44,12 +44,12 @@ describe('new UniversalRouter(routes, options)', () => {
     expect(result).toBe('result')
     expect(errorHandler.mock.calls.length).toBe(1)
     const error = errorHandler.mock.calls[0][0]
+    const context = errorHandler.mock.calls[0][1]
     expect(error).toBeInstanceOf(Error)
-    expect(error.message).toBe('Page not found')
-    expect(error.code).toBe(404)
-    expect(error.context.pathname).toBe('/')
-    expect(error.context.path).toBe(undefined)
-    expect(error.context.router).toBe(router)
+    expect(error.message).toBe('Route not found')
+    expect(error.status).toBe(404)
+    expect(context.pathname).toBe('/')
+    expect(context.router).toBe(router)
   })
 
   it('should handle route errors', async () => {
@@ -65,13 +65,13 @@ describe('new UniversalRouter(routes, options)', () => {
     expect(result).toBe('result')
     expect(errorHandler.mock.calls.length).toBe(1)
     const error = errorHandler.mock.calls[0][0]
+    const context = errorHandler.mock.calls[0][1]
     expect(error).toBeInstanceOf(Error)
     expect(error.message).toBe('custom')
-    expect(error.code).toBe(500)
-    expect(error.context.pathname).toBe('/')
-    expect(error.context.path).toBe('/')
-    expect(error.context.router).toBe(router)
-    expect(error.context.route).toBe(route)
+    expect(context.pathname).toBe('/')
+    expect(context.path).toBe('/')
+    expect(context.router).toBe(router)
+    expect(context.route).toBe(route)
   })
 })
 
@@ -85,11 +85,8 @@ describe('router.resolve({ pathname, ...context })', () => {
       err = e
     }
     expect(err).toBeInstanceOf(Error)
-    expect(err.message).toBe('Page not found')
-    expect(err.code).toBe(404)
-    expect(err.context.pathname).toBe('/')
-    expect(err.context.path).toBe(undefined)
-    expect(err.context.router).toBe(router)
+    expect(err.message).toBe('Route not found')
+    expect(err.status).toBe(404)
   })
 
   it("should execute the matching route's action method and return its result", async () => {
@@ -140,8 +137,8 @@ describe('router.resolve({ pathname, ...context })', () => {
       err = e
     }
     expect(err).toBeInstanceOf(Error)
-    expect(err.message).toBe('Page not found')
-    expect(err.code).toBe(404)
+    expect(err.message).toBe('Route not found')
+    expect(err.status).toBe(404)
     expect(action.mock.calls.length).toBe(0)
   })
 
@@ -545,11 +542,8 @@ describe('router.resolve({ pathname, ...context })', () => {
     }
     expect(action.mock.calls.length).toBe(1)
     expect(err).toBeInstanceOf(Error)
-    expect(err.message).toBe('Page not found')
-    expect(err.code).toBe(404)
-    expect(err.context.pathname).toBe('/a/b/c')
-    expect(err.context.path).toBe(undefined)
-    expect(err.context.router).toBe(router)
+    expect(err.message).toBe('Route not found')
+    expect(err.status).toBe(404)
   })
 
   it('should match routes with trailing slashes', async () => {