> > I am proposing that we really look at what the requirement is here, and > come up with something that fits the bill properly, rather than rehash > previous principles that smell bad. > > [...] `interface MiddlewareInterface {}` [...] > > We need a base interface that can allow type hinting a single point of > adding middleware to a queue for example. >
An empty interface that exists only to extend other interfaces from to me smells bad. (The spec used to have one <https://github.com/http-interop/http-middleware/commit/45191131b718d4afaa8789a8d5cacba410f7810e> when it was trying to do both client and server-side middlewares; it smelt bad there too.) It suggests to me that the concerns you are trying to separate are not in fact separate, but part of the same concern (the concern of middleware: take a request, optionally using a delegate to get a response, return a response). YMMV. We are violating the SRP and ISP in both implementations thus far, and we > need to reimagine this as above to prevent that violation. Let me write the > middleware use cases I've seen so far: > > 1. I want to make a Middleware to handle requests. > 2. I want to make a Middleware to alter a response. > 3. I need to make an instance of a response to short circuit the > exchange. > > 4. I want to make a Middleware that alters a response according to the handled request. An example using the currently proposed PSR-15 interfaces: class HttpResponsePreparingMiddleware implements MiddlewareInterface { public function process( ServerRequestInterface $request, DelegateInterface $delegate ): ResponseInterface { /** Do nothing to the request, hand it on to later middleware. */ /** @var ResponseInterface $response Response received from delegate. */ $response = $delegate->process($request); /** Check response content is acceptable to recipient. */ if ($request->hasHeader('Accept')) { $acceptable = $request->getHeader('Accept'); $sending = $response->getHeader('Content-Type'); if(!in_array($sending[0], $acceptable)) { // `buildErrorResponse()` is not shown here for brevity. $response = $this->buildErrorResponse(406); } } /** Ensure the HTTP protocol is acceptable to recipient. */ $requestVersion = $request->getProtocolVersion(); if ($response->getProtocolVersion() != $requestVersion) { $version = ($requestVersion == '1.1') ? '1.1' : '1.0'; $response = $response->withProtocolVersion($version); } /** Return a request I know the client will accept. */ return $response; } } This middleware has one concern: ensure a valid response is returned, for a given request. It needs the request to do that, so should use your `RequestMiddlewareInterface`, but it also needs a response and has no delegate to demand one from and my return type is wrong. So it should use `ResponseMiddlewareInterface` instead? Now I have the right return type and one of my dependencies, but I don't have a request. So I then implement both interfaces: `parseRequest()` stores the request in a property and returns it; `parseResponse()` picks up the stored request and uses it. But why are these two methods? I only have one concern. These are fundamentally three separate responsibilities, and must be > separated. There is no valid argument against this, sorry to annoy anyone > there, but that's the truth. To fix this from both current implementations, > we segregate the interfaces, therefore separating the concerns. > No, it's not the truth. No, we don't need to artificially divide a single concern. The concern of middleware is to return a response from a given request. It does that by optionally delegating to a later middleware to perform its concern. You seem to object to the fact that a middleware is aware that it exists in an environment and has a delegate to call upon. I don't understand why: the middleware design pattern is predicated on there being a linear flow of control between one or more middleware instances both in (for a request) and out (for a response). If you try to build middleware that is unaware of that fact of its design, then it stops being middleware. It is instead a request or response handler. Also, `RequestMiddlewareInterface` having a return hint of `MessageInterface` is for the express purpose of short circuiting the stack. It is giving the middleware control over the stack, just as the option to call or not `$delegate->process()` does. You can't claim to remove middleware's awareness/degree of control of the stack and keep it by stealth. Something akin to what Josh suggested, a very simple middleware: class ThrowableCatchingMiddleware implements MiddlewareInterface { public function process( ServerRequestInterface $request, DelegateInterface $delegate ): ResponseInterface { try { $this->logger->debug('Request received', ['request' => $request ]); $response = $delegate->process($request); $this->logger->debug('Response sent', ['response' => $response ]); } catch (Throwable $caught) { // `buildErrorResponse()` builds a valid HTTP error response for a // given HTTP error code. If the given code is not a recognised HTTP // error code, 500 is used. $response = $this->buildErrorResponse($caught->getCode()); $this->logger->error( 'Exception encountered', ['response' => $response, 'exception' => $caught] ); return $response; } } } ... Which apparently can't exist under you proposal. Because the proposal makes some sweeping assumptions and dictates implementation details: The stack is not the top level of an application, it is to be used inside > an application; the app itself should be responsible for final output > whether that be a true response or a handled exception. > I can imagine a light weight application where the top level *is *the stack and all the functionality is plugged into it as middleware. Since web frameworks are toolkits for turning a HTTP request into a response, this doesn't seem particularly outlandish. In fact, it seems a particularly likely use case for the spec. Exception catching/logging middleware may not be the best design choice outside of such an app (and, actually, I'm not even sure about that), but that is no reason to deny an end user the choice of using it. Interop is all about letting solutions from a broad set of use cases work the same way, right? The spec should promote good practice, yes. But it should not define good practice, especially in such a narrow and opinionated way. On Thursday, February 23, 2017 at 10:24:59 AM UTC, John Porter wrote: > > Hi Josh, > > I've been having a good old think about what you asked, as it confused me > a little being that I don't code that way myself. > > To me, exception handling, to be effective, should happen at your > application level, not at such a low level as middleware; in my > applications, the function calling $stack->exchange(...); would be wrapped > in an application wide exception handler which would return a response > based on that. > > I'm concerned here for splitting out responsibilities in code, so that the > middleware are responsible for creating a response, and if they can't, > throw an exception. The stack is not the top level of an application, it is > to be used inside an application; the app itself should be responsible for > final output whether that be a true response or a handled exception. > > If we had a router at the centre of the stack that talked to a domain, > then of course that could catch any domain exceptions and translate them > into a response or even throw an application level exception. > > On Wednesday, 22 February 2017 11:17:37 UTC, John Porter wrote: >> >> Please point me to examples that I can replicate, otherwise I can't >> answer your questions accurately. >> >> Thanks >> >> On Wednesday, 22 February 2017 11:13:26 UTC, Josh Di Fabio wrote: >>> >>> Hi John, >>> >>> How would you implement a middleware which catches an exception thrown >>> by an inner middleware and returns a response instead of allowing the >>> exception to bubble up? >>> >>> On Wed, Feb 22, 2017 at 11:06 AM Michael Mayer <mic...@schnittstabil.de> >>> wrote: >>> >>>> John, would you mind pushing your interfaces and Stack to github? Thus, >>>> everybody interested can play with your code, which probably would reduce >>>> mails here. >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "PHP Framework Interoperability Group" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to php-fig+u...@googlegroups.com. >>>> To post to this group, send email to php...@googlegroups.com. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/php-fig/a2156982-48f3-424f-b14a-4cb81a6e5335%40googlegroups.com >>>> >>>> <https://groups.google.com/d/msgid/php-fig/a2156982-48f3-424f-b14a-4cb81a6e5335%40googlegroups.com?utm_medium=email&utm_source=footer> >>>> . >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> On Thursday, February 23, 2017 at 10:24:59 AM UTC, John Porter wrote: > > Hi Josh, > > I've been having a good old think about what you asked, as it confused me > a little being that I don't code that way myself. > > To me, exception handling, to be effective, should happen at your > application level, not at such a low level as middleware; in my > applications, the function calling $stack->exchange(...); would be wrapped > in an application wide exception handler which would return a response > based on that. > > I'm concerned here for splitting out responsibilities in code, so that the > middleware are responsible for creating a response, and if they can't, > throw an exception. The stack is not the top level of an application, it is > to be used inside an application; the app itself should be responsible for > final output whether that be a true response or a handled exception. > > If we had a router at the centre of the stack that talked to a domain, > then of course that could catch any domain exceptions and translate them > into a response or even throw an application level exception. > > On Wednesday, 22 February 2017 11:17:37 UTC, John Porter wrote: >> >> Please point me to examples that I can replicate, otherwise I can't >> answer your questions accurately. >> >> Thanks >> >> On Wednesday, 22 February 2017 11:13:26 UTC, Josh Di Fabio wrote: >>> >>> Hi John, >>> >>> How would you implement a middleware which catches an exception thrown >>> by an inner middleware and returns a response instead of allowing the >>> exception to bubble up? >>> >>> On Wed, Feb 22, 2017 at 11:06 AM Michael Mayer <mic...@schnittstabil.de> >>> wrote: >>> >>>> John, would you mind pushing your interfaces and Stack to github? Thus, >>>> everybody interested can play with your code, which probably would reduce >>>> mails here. >>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "PHP Framework Interoperability Group" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to php-fig+u...@googlegroups.com. >>>> To post to this group, send email to php...@googlegroups.com. >>>> To view this discussion on the web visit >>>> https://groups.google.com/d/msgid/php-fig/a2156982-48f3-424f-b14a-4cb81a6e5335%40googlegroups.com >>>> >>>> <https://groups.google.com/d/msgid/php-fig/a2156982-48f3-424f-b14a-4cb81a6e5335%40googlegroups.com?utm_medium=email&utm_source=footer> >>>> . >>>> For more options, visit https://groups.google.com/d/optout. >>>> >>> -- You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group. To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+unsubscr...@googlegroups.com. To post to this group, send email to php-fig@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/64fdc650-6a12-4387-9de8-b7d5b132e386%40googlegroups.com. For more options, visit https://groups.google.com/d/optout.