Error handler with context

CHANGELOG.md

@@ -5,6 +5,20 @@ 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]
+
+- [BREAKING] The router no longer mutates errors to avoid issues with non-extensible objects.
+  ([#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 the same variables using `errorHandler` option:
+  ```js
+  errorHandler(error, context) {
+    const code = error.message === 'Route not found' ? 404 : 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.message === 'Route not found'
       ? '<h1>Page Not Found</h1>'
       : '<h1>Oops! Something went wrong</h1>'
   }

src/UniversalRouter.js

@@ -10,6 +10,8 @@
 import pathToRegexp from 'path-to-regexp'
 import matchRoute from './matchRoute'
 
+const notFound = 'Route not found'
+
 function resolveRoute(context, params) {
   if (typeof context.route.action === 'function') {
     return context.route.action(context, params)
@@ -74,10 +76,7 @@ class UniversalRouter {
       }
 
       if (matches.done) {
-        const error = new Error('Page not found')
-        error.context = context
-        error.code = 404
-        return Promise.reject(error)
+        return Promise.reject(new Error(notFound))
       }
 
       currentContext = { ...context, ...matches.value }
@@ -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,11 @@ 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(context.pathname).toBe('/')
+    expect(context.router).toBe(router)
   })
 
   it('should handle route errors', async () => {
@@ -65,13 +64,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 +84,7 @@ 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')
   })
 
   it("should execute the matching route's action method and return its result", async () => {
@@ -140,8 +135,7 @@ 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(action.mock.calls.length).toBe(0)
   })
 
@@ -545,11 +539,7 @@ 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')
   })
 
   it('should match routes with trailing slashes', async () => {