Allow for identifying 404 vs. 405

[whitlockjc]

Whenever router fails to match a request, it can be one of two reasons:

1. Method mismatch (405)
2. Path mismatch (404)

I cannot seem to figure out a way to differentiate between the two in my finalhandler and I was hoping that either someone can point me in the right direction or maybe router can be updated to allow for such things.

[wesleytodd]

AFAIK the router package does not support this. Looking through the code I think it would drastically change the way things are done to be able to support it. Right now, if the method does not match, then it actually skips the whole path matching. This is a perf improvement because the path matching is the least efficient part.

I think (although have not tried) that you could rig up your routes in such a way as you can do this manually, like:

router.route('/foo')
  .get(getHandler)
  .post(postHandler)
  .all(function (req, res) {
    res.status(405).send();
  });

But you would have to be careful that nothing else after was supposed to handle the route.

[whitlockjc]

Why does it have to be a drastic change? Looking at the source, it seems very simple. You could add a property to err based on whether the miss was due to method and/or path and let the finalhandler use this property to return a 404 or a 405. In fact, I'll submit a PR for this.

[whitlockjc]

I am aware now that this would be a breaking change and that isn't ideal. And since the test suite relies heavily on the current support...this could be bigger than expected. But the logic for this seems pretty simple.

[wesleytodd]

I didn't mean the code change was drastic, rather that the effect of the change would be. Since it is quick I don't see a reason not to submit a PR, but I am not sure I am in support of testing route matching when the method does not match.

[whitlockjc]

I thought about wrapping this up in a configuration option but after implementing this, there is no way to make it work. For a method/path miss, there is no err sent to the done callback right now. I could add one for this but that would change the current handling so using a configuration option alone isn't enough. I am not sure sending an err downstream is such a bad idea but it is a deviation from the current functionality and my concern would be breaking backward compatibility.

I'll leave this open for now but if you think it's not worth considering further, I'm cool if you close it. I do think it's a good idea to be able to differentiate between 404 and 405.

[wesleytodd]

I do think it's a good idea to be able to differentiate between 404 and 405

On this I agree completely.

Sending an err when a route misses is not something we can ever do, because it is not an error, as it could just be handled further down the route stack. There have been a few discussions around tracking the matched layers, which might enable this handling in a more generic way. See #34 & #56. Both of which might enable you to build this feature as a third party. Also there is still the option I listed above using the route and .all methods.

[whitlockjc]

I'll give the .all option you suggested earlier a try and if it works, we can maybe get the docs updated to suggest doing this?

[wesleytodd]

LMK if #63 works for you.

[whitlockjc]

Yep, it worked great! Thanks a ton.