search

varspool / Wrench

A simple PHP WebSocket implementation for PHP 7.1
Do What The F*ck You Want To Public License
596 stars 210 forks source link

Exceptions Galore #14

Closed garyns closed 12 years ago

garyns commented 12 years ago

Hi,

I've moved from php-websockets to Wrench today, but I am in a sea of exceptions and errors now. I though you might like some feedback. Overall I love the concept of what I see and I look forward to using it in our web app to replace the ajax polling we are currently using.

By contract, php-websockets had none of these problems. I attempted to upgrade because we could never get WSS:// working with php-websockets.

We're developing on MAC, and testing with Safari 5.1.7, Chrome 20.0.1132.57 and FireFox 14.0.1. Difference browsers give us different errors.

Firstly, the we found when we start the server as below, it failed as it resolved localhost to ip6 ::1 hence ::1:8000 failed the host/port parsing.

server = new \Wrench\Server('ws://localhost:8000/', array(

We tested the connection in the JS console for each browser with the following code:

var ws = new WebSocket("ws://127.0.0.1:8000/echo");

Here are the results for each browser:

FireFox:

php wsserver.php info: Server initialized info: Wrench\ConnectionManager: Wrench\Connection: 127.0.0.1:60868 (380b0ff3ab4faecd6484fd92df6d998eedbe93aa279d34a244b3b4beba25a70dc1c1391cb3ec1636ccf32a98e5d335b65235e85da086182f4108e83701ceed6a): Connected err: Wrench\ConnectionManager: Error on client socket: exception 'Wrench\Exception\BadRequestException' with message 'Invalid connection header' in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Protocol/Protocol.php:452 Stack trace:

0 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php(206): Wrench\Protocol\Protocol->validateRequestHandshake('GET /echo HTTP/...')

1 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php(189): Wrench\Connection->handshake('GET /echo HTTP/...')

2 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php(402): Wrench\Connection->onData('GET /echo HTTP/...')

3 /Users/lyndoch/dev/TokCam/php/lib/Wrench/ConnectionManager.php(232): Wrench\Connection->process()

4 /Users/lyndoch/dev/TokCam/php/lib/Wrench/ConnectionManager.php(163): Wrench\ConnectionManager->processClientSocket(Resource id #20)

5 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Server.php(187): Wrench\ConnectionManager->selectAndProcess()

6 /Users/lyndoch/dev/TokCam/php/wsserver.php(48): Wrench\Server->run()

7 {main}

PHP Notice: Undefined variable: e in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php on line 415

Notice: Undefined variable: e in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php on line 415

Chrome:

php wsserver.php info: Server initialized info: Wrench\ConnectionManager: Wrench\Connection: 127.0.0.1:60873 (6655c186e8eaa74e24618d140969f1c75de915da9aa81d53a6bde44c596948a481a26ff7fd0aa0d8ef9cd921805536fb7335e9ee7799ae2b976c0d1a4ad1e1e1): Connected info: Wrench\ConnectionManager: Wrench\Connection: 127.0.0.1:60873 (6655c186e8eaa74e24618d140969f1c75de915da9aa81d53a6bde44c596948a481a26ff7fd0aa0d8ef9cd921805536fb7335e9ee7799ae2b976c0d1a4ad1e1e1): Handshake successful: 127.0.0.1:60873 (6655c186e8eaa74e24618d140969f1c75de915da9aa81d53a6bde44c596948a481a26ff7fd0aa0d8ef9cd921805536fb7335e9ee7799ae2b976c0d1a4ad1e1e1) connected to /echo PHP Fatal error: Call to undefined method Wrench\Application\EchoApplication::onConnect() in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php on line 243

Fatal error: Call to undefined method Wrench\Application\EchoApplication::onConnect() in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php on line 243

Safari:

php wsserver.php info: Server initialized info: Wrench\ConnectionManager: Wrench\Connection: 127.0.0.1:60879 (4955168f01e40bd1a0a23bde1c2a20aa9bfc7f34d83f9fffcfcf6cfc3820c5908bd89bfbfa082710d4a63d32fa1fe4c79cb04e3ffb94d33752102a11fc9c5f4a): Connected err: Wrench\ConnectionManager: Error on client socket: exception 'Wrench\Exception\BadRequestException' with message 'Invalid upgrade header' in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Protocol/Protocol.php:446 Stack trace:

0 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php(206): Wrench\Protocol\Protocol->validateRequestHandshake('GET /echo HTTP/...')

1 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php(189): Wrench\Connection->handshake('GET /echo HTTP/...')

2 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php(402): Wrench\Connection->onData('GET /echo HTTP/...')

3 /Users/lyndoch/dev/TokCam/php/lib/Wrench/ConnectionManager.php(232): Wrench\Connection->process()

4 /Users/lyndoch/dev/TokCam/php/lib/Wrench/ConnectionManager.php(163): Wrench\ConnectionManager->processClientSocket(Resource id #20)

5 /Users/lyndoch/dev/TokCam/php/lib/Wrench/Server.php(187): Wrench\ConnectionManager->selectAndProcess()

6 /Users/lyndoch/dev/TokCam/php/wsserver.php(48): Wrench\Server->run()

7 {main}

PHP Notice: Undefined variable: e in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php on line 415

Notice: Undefined variable: e in /Users/lyndoch/dev/TokCam/php/lib/Wrench/Connection.php on line 415

Regards, Gary.

dominics commented 12 years ago

The Firefox support is lacking. I'll make it my next priority (although, eventually I'd like to solve this "properly" by putting in place automated testing).

As for the exception in Chrome, this was a violation of the public contract for Applications: the Connection (wrongly) assumed every Application would define onConnect(), and EchoApplication didn't (because it has no need to listen for connections).

There was also bad handling of sending the close frame on an exception. Both should be fixed in master.

dominics commented 12 years ago

OK, Firefox support should be restored by b8dba6e