http-interop / http-middleware

PSR-15 interfaces for HTTP Middleware
MIT License
73 stars 7 forks source link

Middleware for asynchronous processing? #20

Closed dbu closed 8 years ago

dbu commented 8 years ago

I looked over the spec draft and noticed that it is only useable with blocking requests. On server side, asynchronous frameworks like ReactPHP are not that common. But client side can profit a lot from asynchronous requests. In the PHP-HTTP client, we defined a different signature for plugins. the main difference is that the plugin does not return a response, but a promise:

https://github.com/php-http/client-common/blob/master/src/Plugin.php

IMO it would be wrong to define the promise in this PSR. But the two concepts are incompatible, so if PSR-15 is defined like this, it will become a problem for future improvements. The annoying thing is that a promise approach works without any problems with a synchronous request - the initial caller just waits on the promise and returns the result.

I don't know the state of a promise PSR in the FIG - and unless that can happen soon, it would be unfortunate to block PSR-15 by that. Maybe it would make sense to make PSR-15 about server side only, and wait with a client side PSR until promises are defined?

mindplay-dk commented 8 years ago

Async only matters for outgoing requests, correct?

Would it problematic to add something like an AsyncMiddlewareInterface at a later time?

Essentially treating async middleware as a completely separate type of middleware - much in the same way that middleware and server-middleware are already being treated as completely separate types.

dbu commented 8 years ago

for incoming requests, see for example http://reactphp.org/ . this is not a huge thing yet, but the topic of non-blocking async servers is likely to become more relevant in the future.

but i agree that for most people, async outgoing requests are more important. yes, we could add async handling later. the thing is that the async pattern works well for sync requests, but the opposite is not true. so if we could define async middleware, we would not need to define sync middleware. if we define sync now, it will be hard to remove and we complicate the future life of everybody by having a sync definition that is not relevant anymore.

dbu commented 8 years ago

if we would go with your approach, we should promote to write middleware like this:

class MyMiddleware implements ...
{
    public function process(...)
    {
        try {
            return $this->updateResponse(
                $next->next($this->updateRequest($request))
            );
        } catch ... 
             return $this->handleException($e)
        }
    }
}

and later can do

$promise->then(
  [$this, 'updateResponse'],
  [$this, 'handleException']
);
mindplay-dk commented 8 years ago

The problem is that all of this depends on an as-of-yet non-existent specification.

Perhaps it would be best to cover this in an entirely separate specification for asynchronous middleware in the future, when the time comes?

For developers writing synchronous applications with stacks of purely synchronous middleware, I don't think we need to burden them with the overhead of promises? Now, or in the future.

Adding support for asynchronous middleware to existing PSR-15 dispatchers at a later time, if such a specification is added, should be no problem at all.

dbu commented 8 years ago

The problem is that all of this depends on an as-of-yet non-existent specification.

yep, i know :-(

For developers writing synchronous applications with stacks of purely synchronous middleware, I don't think we need to burden them with the overhead of promises? Now, or in the future.

this is true when you write your own middleware. however, if you provide reusable middleware like the generic "plugins" we have at https://github.com/php-http/client-common/tree/master/src/Plugin , its a different matter. of course, we could extract the logic into a decoupled class and write a "plugin adapter" for sync and later for async requests. but the overhead of an application with several middleware is already considerable. adding another layer to every middleware sounds bad.

Adding support for asynchronous middleware to existing PSR-15 dispatchers at a later time, if such a specification is added, should be no problem at all.

i don't see how that would be possible. the contract in the current PSR-15 proposal says that the process method accepts a request and returns a response. while i think its the right way to do middleware, it prevents asynchronity completely. the request must go through the process method to possibly be updated, and the response must go through process to possibly be updated. this means having a single non-async middleware would break the chain for async.

shadowhand commented 8 years ago

I think async middleware is an entirely different topic, and as such should be defined by a different PSR, or be worked into the Async PSR if PSR-15 is ratified in a timely fashion.

mindplay-dk commented 8 years ago

Adding support for asynchronous middleware to existing PSR-15 dispatchers at a later time, if such a specification is added, should be no problem at all.

i don't see how that would be possible. the contract in the current PSR-15 proposal says that the process method accepts a request and returns a response. while i think its the right way to do middleware, it prevents asynchronity completely.

Not necessarily.

Let's say we have this in PSR-15:

interface MiddlewareInterface {
    process(RequestInterface $request, DelegateInterface $delegate): ResponseInterface;
}

interface ServerMiddlewareInterface {
    process(ServerRequestInterface $request, DelegateInterface $delegate): ResponseInterface;
}

In PSR-15, these are mutually exclusive - you can only implement one or the other, as indicated by then identical method-names with different signatures.

A future PSR-X for asynchronous middleware could use a different method-name:

interface AsyncMiddlewareInterface {
    processAsync(RequestInterface $request, AsyncDelegateInterface $delegate): ResponsePromiseInterface;
}

Middleware would need to explicitly support deferred processing. That's not necessarily a bad thing, but it's not the only option.

The other option is to replace the interfaces, in which case the future PSR would be mutually exclusive with this one - that is, a middleware developer would need to support either one or the other, and a future dispatcher that supports asynchronous middleware could support both PSR-15 and PSR-X, e.g. by internally wrapping PSR-15 responses in immediately-resolved Promises.

Either way, a PSR for Promises doesn't exist yet, and even then, the PSR for async middleware is even farther off - so PSR-15 will have to move ahead without this feature.

I'm confident we can find a practical solution if/when the need arises.

dbu commented 8 years ago

It will require refactoring every plugin but i agree with your reasoning to not delay. Is there a place we could mention this potential future evolution, along the lines of https://github.com/http-interop/http-middleware/issues/20#issuecomment-247922712 ?

mindplay-dk commented 8 years ago

Is there a place we could mention this potential future evolution

We could mention it in the meta, though I'm not sure there would be much of a point to doing so? We're talking about a different type of middleware - it wouldn't be a new version of this PSR, because that would be a breaking change, so it would be a new PSR, either supplemental or stand-alone.

@shadowhand if mentioned in the meta, we could simply state that it was discussed and rejected on the grounds that there is no PSR for Promises at this time?

dbu commented 8 years ago

i would appreciate stating its not in because promises where not available at the time of writing. just to avoid confusion once promises are here.