Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork9.7k
[HttpClient] Double check if handle is complete#44479
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
divinity76 commentedDec 7, 2021
+1 from me, btw wonder if a E_USER_WARNING about an unknown-ignored message would be appropriate (probably means you're running an old version of symfony) another thing, the code above seems faulty, the loop above should be changed from to something like while exactly how to handle the error may be up for debate, just ignoring the error (as the current code does) is not the right approach. i've done it like do {$err =curl_multi_exec($mh,$still_running); }while ($err ===CURLM_CALL_MULTI_PERFORM);if ($err !==CURLM_OK) {$errinfo = ["multi_exec_return" =>$err,"curl_multi_errno" =>curl_multi_errno($mh),"curl_multi_strerror" =>curl_multi_strerror($err) ];$errstr ="curl_multi_exec error:" .var_export($errinfo,true);thrownew \RuntimeException($errstr); } in the past, but didn't really give it much thought as to how it should really be handled. possible return values include |
Nyholm commentedDec 7, 2021
Im not sure what you mean. Are you suggesting we should trigger a Your other comments seams interesting. Feel free to open an new PR or issue about those. They are a bit unrelated to this PR. |
divinity76 commentedDec 7, 2021
maybe, after all, it means that this version Symfony received a message from libcurl that this version of Symfony doesn't understand, and because the message wasn't understood, Symfony opted to completely ignore the message. does that warrant a warning? i don't know tbh. i don't have strong feelings on it either way, just ignoring unknown messages is probably fine.
yeah maybe i'll do that ^^ |
50ddbe2 tof35a6adComparenicolas-grekas commentedDec 11, 2021
Thank you@Nyholm. |
Uh oh!
There was an error while loading.Please reload this page.
This is a forward compatibility fix. We did the same in Guzzle after a comment from bagder.
guzzle/guzzle#2892 (comment)
Basically, if libcurl decides to add an other value for
msg, then our code will break without this PR. The only value formsgwith current latest version of curl isCURLMSG_DONEwhich means that this is not a bugfix yet.. but it make sure that we respect the libcurl API.