Undefined route handling

[allspiritseve]

If I make a request for a route that has not been defined, e.g. GET /unknown, I get a 404 response with Cannot GET /unknown but the request does not show up in the logs.

I would like to discuss the following additions:

1. Log undefined routes
2. Optionally define a custom handler for undefined routes (e.g. perhaps I want to return a JSON body instead of plain text)

[bigeasy]

Question is whether we want to build all this into the Reactor middleware or allow it to be compostable which is the way Connect seems think it's supposed to work. Connect does have a default error handler. The Connect behavior should be to forward errors to an error handler, or invoke the next handler if routes don't match.

The decision to return 404 and plain text was a decision to simplify construction, not make Reactor so Sencha Connecty.

[allspiritseve]

Actually, I was able to accomplish the above without changes to reactor:

var MyService = function(config) {
  this.reactor = new Reactor(this, function(reactor) {
    reactor.dispatch('GET /ping', 'ping')
    reactor.dispatch('.*', 'notFound')
  })
}

// ...

MyService.prototype.notFound = cadence(function() {
  return [{ message: 'Not Found' }, 404]
})

Maybe that is sufficient?

[bigeasy]

Huh. Sure. That'll do it. With that in mind, maybe we make that option somehow documented and official.

You use Reactor as if it was a Ruby on Rails router and not as though it where one of many bits of Sencha Connect middleware composed into an application. This is generally how it's been useful in our work, with the default pipeline built by Reactor. If we tend to handle our own 404's we could define it this way, maybe with a helper to the constructor. If someone else wants to do it the Sencha Connect way they do their own middleware pipeline assembly and make the next bit of middleware in the pipeline the error/missing handler.

Except how do we customize handling errors as well?

[bigeasy]

404s do go to the next bit of middleware anyway. We handle errors. The only thing that you would possibly want to do is maybe create specify an error handler.

[allspiritseve]

Well, I thought you wanted to avoid the connect way with lots of middlewares. If we're looking for interesting patterns, I like Sinatra's not_found and error handlers. A reactor equivalent might look like:

reactor.notFound('notFound')
reactor.error(400, 'badRequest')

[bigeasy]

I like your find for not found. That covers it. It's actually valid Sencha Connect, I think. The question is customizing error response. Here's the Sencha Connect error handler. It answers my next question which would be, what about accept headers?

http://www.senchalabs.org/connect/errorHandler.html

I created this specialized error handling because the default handling returned HTML and too much of it. I didn't like that it returned a stack trace. I feel that should be logged but not leave the server.

But yeah, missing routes (404s) and errors are two separate issues.

[bigeasy]

Sketch of specified error handler.

54

[allspiritseve]

I checked the Sinatra code, and not_found is actually a special case of the error handler for 404 status codes or Sinatra::NotFound exceptions. The error handler can match status codes or exceptions (there's a bit of hackery with exceptions to add a http_status method, which I think defaults to 500).

How hard do you think it would be to expand your sketch to do something like this:

reactor.error('400..599', 'errorHandler')
reactor.error('404', 'notFound')
# with sugar
reactor.notFound('notFound')

Bear in mind I'm just spitballing here. I don't have an immediate need for this.

[bigeasy]

Regarding spitballing, there needs to be some way to have the outcome not be a response from Reactor, but an error forwarded to a subsequent bit of middleware. If you look at the Sencha Connect code, you can see that it will set an error status from a status in the error message.

Really, this is a pretty terrible way to do our error handling (what about error messages? what about additional headers?), but I still believe in having a path toward being a good Sencha Connect citizen, not walling it off.

Or maybe the error handler is new middleware itself? (Not a terribly exciting idea for us, but something to consider.)

The problem is that an Error is not accustomed to taking on a lot of context, extra properties. Do you forward those using an Error with Error.headers or do you set a property in request which is also forwarded, or if you are doing something like a 301, do you set headers by calling response.setHeader?

Having said all that, I'm starting to see the default Sencha Connect error handler as being a minimal catch all and probably not useful for our applications. They had to have something to get people started, though. I can see that you might want requests that don't match the paths to pass through Reactor middleware and onto other middleware that matches other paths, but I don't see where you'd want to have Reactor middleware emit errors for some other middleware to format them. The default Sencha Connect error handler is not some canonical middleware that we're all supposed to default to.

Reactor's exception based errors are one of it's more useful features.