check return value of stream_select #20
Conversation
|
Example: some framework (like yii) converts all errors to exceptions. |
|
@holycheater That's true, but not the target of this PR. There has been a pull request to suppress these warnings (see reactphp/reactphp#297 ). Like @cboden said, error suppression would not be included in the core due to performance reasons. I solved this by using my custom EventLoop-Implementation. |
|
Can you provide some example code so that I could see the error you see (before the PR change)? |
|
Sorry for the delay, I've created a repo with a small showcase-app here. If you run run-me.php and kill it afterwards, a warning is shown. |
|
Changes LGTM, however I'm curious to see if we can come up with a reproducible test case 👍 FWIW: The Travis errors are unrelated (network hiccup). |
|
Changes LGTM but same as @clue I wonder IF we can come up with a test case 👍 |
|
|
|
actually I don't recall why I named it $n. What name do you suggest? |
|
@mkrauser considering that |
|
@nrk Ok, personally I'm using $n for numbers, but $available is indeed more readable. I've changed it |
adds a unit test for reactphp#20
|
Closing in favour of #44 being merged |
This change was already requested as part of reactphp/reactphp#297
In the current Implementation, the return value of stream_select is not checked at all. According to the php documentation, a return value of
falseindicates that the underlying system call failed, e.g. due to a received signal. If thats the case, we cannot savely rely on the outcome of stream_select.Unlike reactphp/reactphp#297, this PR does not supress any raised warning.