iron-meteor / iron-router

A client and server side router designed specifically for Meteor.
MIT License
1.98k stars 413 forks source link

Global error-handling requires preserving handler arity. #1477

Open GeorgeCalvert opened 8 years ago

GeorgeCalvert commented 8 years ago

The changes here open the can of worms that is error handling, Router.use etc. :-) I offer it more as food for thought than with an expectation that it's what's needed long-term.

This is the minimal change that allowed us to create a catchall route (as part of a REST API) that would convert thrown errors into a useful reply to the client. The issue was that setting the route itself as the handler meant dispatch always saw a 3-arity function and so never invoked the error handler. These changes work because the catchall route applies to all verbs.

Currently of course, the route.get et al add their handlers to the route's action stack. While I see the benefits of that in relation to defining before- and after-action handlers, a 2D stack structure requires I hook a error handler on every route.

Granted I haven't spent much time with the client side -- yet :-) -- but, for me, it'd be cleaner to have just one linear stack of routes. I understand we want handling a route to, in fact, walk a set of handlers in order to cover arbitrary sets of before- and after-hooks. What that requires, though, is some way of nexting within the route or across routes (a la Express adding next('route') into the mix).

GeorgeCalvert commented 8 years ago

When integrating the error-handling catchall with the client, I needed to modify line 203 in lib/router.js to if (route && route.createController). To me, while the changes in this request might be sufficient, they are suggestive of making a deeper overhaul.

I look forward to some discussion on the topic :-)