Provide cancellation token source to push handler #245
Conversation
| @@ -594,7 +594,7 @@ static function () { | |||
| $onPush = $stream->request->getPushHandler(); | |||
|
|
|||
| try { | |||
| yield call($onPush, $stream->request, $stream->pendingResponse->promise()); | |||
| yield call($onPush, $stream->request, $stream->pendingResponse->promise(), $tokenSource); | |||
There was a problem hiding this comment.
Because of the finally below it seems like it's only possible to cancel while the promise returned from $onPush has not resolved, so it doesn't seem to make anything easier.
There was a problem hiding this comment.
D'oh, yes, that will have to be pushed to the code resolving the promise.
trowski
force-pushed
the
push-cancellation-token
branch
from
9718e94
to
fe17e49
Compare
| $promise = $stream->pendingResponse->promise(); | ||
| $promise->onResolve(function () use ($cancellationToken, $cancellationId): void { | ||
| $cancellationToken->unsubscribe($cancellationId); | ||
| }); |
There was a problem hiding this comment.
This isn't correct either, I guess it has to subscribe to the $response->getTrailers() promise.
There was a problem hiding this comment.
Yep. This keeps getting more complicated. 😛
There was a problem hiding this comment.
Maybe we should just keep the current behavior?
There was a problem hiding this comment.
I'm thinking so. The user can return a promise that isn't resolved until they know they want to receive the push.
@nicolas-grekas, you can return a deferred promise from the handler, subsequently failing the deferred to cancel the push.
There was a problem hiding this comment.
Got it, thank you and sorry for the noise, I should have thought about deferred earlier.
|
Closing due to #245 (comment). |
Related to #243.