Add async support #323
Draft
Add async support #323
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
I guess we could introduce a dependency on a concrete promise library such as https://github.com/reactphp/promise/tree/2.x |
|
I never worked on async/promise on PHP so I might not be able to properly review it. But will it force people to use async/promise or will they still be able to use the sync way like before? |
|
We can provide sync api by just calling |
|
Tests depend on j0k3r/httplug-ssrf-plugin#10 |
jtojnar
force-pushed
the
async
branch
2 times, most recently
from
ade7370
to
5c6ba77
Compare
|
Will want to finish #324 first, since that way conflicts are easier to resolve. |
jtojnar
force-pushed
the
async
branch
4 times, most recently
from
4738de6
to
6b7495b
Compare
It does not support async requests.
It is sync only.
Previous versions broke async clients.
It will still be converted to a dual async/sync one by `PluginClient`.
The diff is ugly but I just: - removed the try block - moved the code after the catch blocks inside the first closure argument of `then` - changed the catch blocks into `if`s with `instanceof` checks - moved them to the second closure argument of `then` - replaced `fetch` with `fetchAsync` Unfortunately, HTTPlug’s `HttpFulfilledPromise` is broken (does not allow to be converted to another promise type and only supports resolving with `ResponseInterface`) so we need the ugly hack in `makePromise` to convert it into a promise object from a full-fledged promise library. I considered ReactPHP’s promise library first but those promises are not natively waitable in version 2.0 and using custom busy-loop waiting or third-party wait (from choval/async) just seemed to block. In the end, I went with Guzzle’s promise library.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Unfortunately, it currently does not work because
HttpFulfilledPromiseis broken (does not allow to be converted to another promise type).