reactphp / http

Event-driven, streaming HTTP client and server implementation for ReactPHP.
https://reactphp.org/http/
MIT License
747 stars 143 forks source link

Random HTTP Requests don't resolve #509

Closed ellisonpatterson closed 11 months ago

ellisonpatterson commented 12 months ago

I am just doing simple GET queries, and for Promises v2, I modified Deferred so I can get logs whenever any promise is rejected. That is all and good, but randomly from time to time, I see that a promise was rejected, but it does not go down the chain and I am unable to handle the rejected promise result.

It looks like it the DNS server randomly does not want to work, but for some reason it's not triggering a rejection handle in the original promise ($browser->get()).

Since I can't capture the promise being rejected (when this issue occurs), I can't retry HTTP queries or handle other errors.

I tried making a test script that uses an invalid DNS IP (like how the DNS library factory method builds it all) but that was resolving the promise and I was able to capture the rejection as intended.

Is there some edge-case that is happening where the original promise doesn't get resolved?

I also am an idiot as well, so that could be the issue.

Thank you!

[2023-11-20 12:16:48] CRITICAL: Deferred rejection occured!
#0 /cliapps/src/Patches/Deferred.php(16): React\Promise\Deferred __construct
#1 /cliapps/vendor/react/dns/src/Query/RetryExecutor.php(26): React\Dns\Query\RetryExecutor tryQuery
#2 /cliapps/vendor/react/dns/src/Query/RetryExecutor.php(21): React\Dns\Query\RetryExecutor query
#3 /cliapps/vendor/react/dns/src/Query/CoopExecutor.php(57): React\Dns\Query\CoopExecutor query
#4 /cliapps/vendor/react/dns/src/Query/CachingExecutor.php(44): React\Dns\Query\CachingExecutor React\Dns\Query\{closure}
#5 /cliapps/vendor/react/promise/src/FulfilledPromise.php(28): React\Promise\FulfilledPromise then
#6 /cliapps/vendor/react/dns/src/Query/CachingExecutor.php(36): React\Dns\Query\CachingExecutor React\Dns\Query\{closure}
#7 /cliapps/vendor/react/promise/src/Promise.php(228): React\Promise\Promise call
#8 /cliapps/vendor/react/promise/src/Promise.php(25): React\Promise\Promise __construct
#9 /cliapps/vendor/react/dns/src/Query/CachingExecutor.php(35): React\Dns\Query\CachingExecutor query
#10 /cliapps/vendor/react/dns/src/Query/HostsFileExecutor.php(64): React\Dns\Query\HostsFileExecutor query
#11 /cliapps/vendor/react/dns/src/Resolver/Resolver.php(34): React\Dns\Resolver\Resolver resolveAll
#12 /cliapps/vendor/react/socket/src/HappyEyeBallsConnectionBuilder.php(130): React\Socket\HappyEyeBallsConnectionBuilder resolve
#13 /cliapps/vendor/react/socket/src/HappyEyeBallsConnectionBuilder.php(85): React\Socket\HappyEyeBallsConnectionBuilder React\Socket\{closure}
#14 /cliapps/vendor/react/promise/src/Promise.php(228): React\Promise\Promise call
#15 /cliapps/vendor/react/promise/src/Promise.php(25): React\Promise\Promise __construct
#16 /cliapps/vendor/react/socket/src/HappyEyeBallsConnectionBuilder.php(70): React\Socket\HappyEyeBallsConnectionBuilder connect
#17 /cliapps/vendor/react/socket/src/HappyEyeBallsConnector.php(67): React\Socket\HappyEyeBallsConnector connect
#18 /cliapps/vendor/react/socket/src/SecureConnector.php(47): React\Socket\SecureConnector connect
#19 /cliapps/vendor/react/socket/src/TimeoutConnector.php(24): React\Socket\TimeoutConnector connect
#20 /cliapps/vendor/react/socket/src/Connector.php(177): React\Socket\Connector connect
#21 /cliapps/vendor/react/http/src/Io/ClientConnectionManager.php(82): React\Http\Io\ClientConnectionManager connect
#22 /cliapps/vendor/react/http/src/Io/ClientRequestStream.php(66): React\Http\Io\ClientRequestStream writeHead
#23 /cliapps/vendor/react/http/src/Io/ClientRequestStream.php(122): React\Http\Io\ClientRequestStream write
#24 /cliapps/vendor/react/http/src/Io/ClientRequestStream.php(135): React\Http\Io\ClientRequestStream end
#25 /cliapps/vendor/react/http/src/Io/Sender.php(152): React\Http\Io\Sender send
#26 /cliapps/vendor/react/http/src/Io/Transaction.php(146): React\Http\Io\Transaction next
#27 /cliapps/vendor/react/http/src/Io/Transaction.php(83): React\Http\Io\Transaction send
#28 /cliapps/vendor/react/http/src/Browser.php(853): React\Http\Browser requestMayBeStreaming
#29 /cliapps/vendor/react/http/src/Browser.php(115): React\Http\Browser get
...
WyriHaximus commented 12 months ago

Hey @ellisonpatterson,

In the HappyEyeBallsConnectionBuilder comes up. Without deeply diving into the problem please know that happy eyeballs attempts mutiple connections if it gets multiples IP's from the DNS server to acquire a connection quickly. If a connection attempt is in flight but another one succeeds that attempt and the problems will be canceled and the rejection from that will be ignored because a successful connection has been created. Do you get the message as well because you should be able to get a lot of information from that.

SimonFrings commented 11 months ago

@ellisonpatterson What @WyriHaximus said, there are a few cases where we internally reject promises and handle them accordingly. I'm not sure how much is gained by modifying the Deferred as it doesn't really matter how many promises we reject internally as long as we're handling them. If you're on the lookout for unhandled promise rejections I'd suggest to upgrade to Promise v3 if possible.

Moreover, I think this kind of ticket is better suited for a discussion in our "Q&A" section, as we recommend opening bug tickets only when you can provide evidence of something being broken. Just as a friendly reminder for future tickets :)

I believe this should answer your question, so I will close this ticket for now :+1: