8000 Make reads and writes to sockets silent with the @-operator by Kekos · Pull Request #2 · php-mqtt/client · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Make reads and writes to sockets silent with the @-operator #2

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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 15, 2020

Conversation

Kekos
Copy link
Contributor
@Kekos Kekos commented Feb 14, 2020

If the MQTT daemon closes the connection before MQTTClient sends a keepalive ping the socket ends up broken and PHP issues a notice. Some frameworks halts the execution on all errors which makes it impossible to catch the DataTransferException and reconnect.

This commit makes all fread and fwrite completely silent and hands over to the already built in error handling in MQTTClient.

@Namoshek
Copy li 8000 nk
Collaborator

I'm not convinced this solution works for all applications. Laravel applications for example have a built-in error handler which does ignore the error suppressing operator @ and will throw anyway.

The standard way to check for closed sockets seems to be using fread($socket, 1) which will return false for closed sockets (not a PHP warning) and an empty string if the connection is still open (and if no data has been sent by the server). The problem with this approach is it requires the non-blocking mode of the socket to be used and currently this library allows using both modes (although I don't know why you'd use the blocking one). It also seems problematic to me that using fread could actually fetch some incoming data when testing the socket for its state, which in return would be missing in readFromSocket() later.

To sum it up, fread($sock) should never throw a PHP warning but fwrite($sock) does. So I'd suggest we only suppress the warning of fwrite to avoid suppressing critical issues unwantedly, at least until we find a better solution (or PHP provides a better one 😄).

@Namoshek Namoshek merged commit 359c5e2 into php-mqtt:0.1 Feb 15, 2020
@Kekos
Copy link
Contributor Author
Kekos commented Feb 15, 2020

Thank you for an interesting clarifying about fread()!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0