NotFoundDelegate and NotFoundHandler

[danizord]

While upgrading to Expressive 2 I found these new NotFoundDelegate and NotFoundHandler a bit confusing. 😕

The 2.0 docs suggests using NotFoundDelegate as default delegate and piping NotFoundHandler as the outermost (EDIT: inner) middleware, which makes no sense for me since NotFoundHandler uses NotFoundDelegate internally and NotFoundDelegate would be called anyway if NotFoundHandler was not piped.

As such, to avoid confusion I'd suggest removing the "default delegate" concept and recommend usage of "catch-all" middlewares only.

Thoughts?

[geerteltink]

piping NotFoundHandler as the outermost middleware

I guess you mean the most inner layer. A request goes from the outside to the inside. It first encounters the ErrorHandler (outermost layer), all other middleware and finally the NotFoundHandler (innermost layer) if none of the previous middleware returned a response.

I'm not sure where you read that, but as I understand it, the NotFoundDelegate is something internally and you don't need to set it up. It's sort of a fail safe in case the NotFoundHandler is not working or missing. If you have a (custom) NotFoundHandler than Expressive should never get to the NotFoundDelegate.

Somehow I agree that it doesn't make sense to have a NotFoundHandler AND a NotFoundDelegate. They basically do the same thing: return a 404 if no response is returned at that point. But than again, if the that delegate/handler doesn't do what it should be doing, the fail safe is gone and you don't get a 404 response.

[danizord]

I guess you mean the most inner layer.

Yeah, sorry. Fixed it :D

If you have a (custom) NotFoundHandler than Expressive should never get to the NotFoundDelegate.

Well, you can provide a custom DefaultDelegate as well.

But than again, if the that delegate/handler doesn't do what it should be doing, the fail safe is gone and you don't get a 404 response.

My point is that we don't need to pipe a NotFoundHandler at all and rely on NotFoundDelegate directly.

[weierophinney]

My point is that we don't need to pipe a NotFoundHandler at all and rely on NotFoundDelegate directly.

I see the argument, however I think there's room for both, mainly because the NotFoundDelegate fulfills the DefaultDelegate service as a default implementation. As such, users could provide a separate default delegate implementation that they use - for instance, one that returns a canned response. Alternately, they could continue to use the NotFoundDelegate as the default delegate, but not use the NotFoundHandler, instead providing something custom that does content negotiation to provide JSON, XML, and HTML responses. Or have it return something other than a 404 (perhaps a 400, indicating a bad request).

Having both explicitly in the pipeline and default services makes it clear to new users that they are separate roles, and can each be customized to provide different behavior. I think that's powerful.

[danizord]

@weierophinney

users could provide a separate default delegate implementation that they use - for instance, one that returns a canned response

Why not providing it as a middleware instead? I think DelegateInterface is not supposed to be implemented in userland.

Alternately, they could continue to use the NotFoundDelegate as the default delegate, but not use the NotFoundHandler, instead providing something custom that does content negotiation to provide JSON, XML, and HTML responses.

Yeah, in this case the NotFoundDelegate will never be reached, which makes it useless. Also I think it is easier to explain to developers that the last middleware should always return a response than explaining that they should register a service aliased as DefaultDelegate.

Or have it return something other than a 404 (perhaps a 400, indicating a bad request).

Again that could be implemented as a middleware. That's why I think there is no place for both here. Since middlewares can return the response without calling the delegate, there's no reason to have a default delegate after all, Zend\Expressive could simply throw an exception with a "no middleware returned a response" message.

[ashish701ranjan]

@danizord

Again that could be implemented as a middleware. That's why I think there is no place for both here. Since middlewares can return the response without calling the delegate, there's no reason to have a default delegate after all, Zend\Expressive could simply throw an exception with a "no middleware returned a response" message.

I agree, why not just have a custom middleware instead of NotFoundHandler. Why is it not recommended in docs instead of asking to create a custom Delegate.

[weierophinney]

Version 3 will no longer have this feature, as of #543, which was merged yesterday. The assumption will be that the middleware pipeline returns a response, and if the queue is exhausted, it will raise an exception.

In that patch, I kept the Handler\NotFoundHandler (renamed from Delegate\NotFoundDelegate) and Middleware\NotFoundMiddleware (renamed from Middleware\NotFoundHandler; however, I will be merging these two into the latter in an upcoming pull request.