amphp / websocket-client

Async WebSocket client for PHP based on Amp.
https://amphp.org/websocket-client
MIT License
147 stars 17 forks source link

Question: Documentation on available `\Amp\Websocket\Options` #18

Closed dfelton closed 5 years ago

dfelton commented 5 years ago

Is there a documentation page somewhere that documents nature of the various \Amp\Websocket\Options properties?

    private $streamThreshold = 32768; // 32KB
    private $frameSplitThreshold = 32768; // 32KB
    private $bytesPerSecondLimit = 1048576; // 1MB
    private $framesPerSecondLimit = 100;
    private $frameSizeLimit = 2097152; // 2MB
    private $messageSizeLimit = 10485760; // 10MB
    private $textOnly = false;
    private $validateUtf8 = true;
    private $closePeriod = 3;
    private $compressionEnabled = false;
    private $heartbeatEnabled = true;
    private $heartbeatPeriod = 10;
    private $queuedPingLimit = 3;

Some of these are rather self-explanatory given their name, but it would be nice to see a page with thorough documentation on them (if one exists).

Thank you!

trowski commented 5 years ago

Sorry, we have not yet put together documentation for the Options class. As you mentioned most are pretty self-explanatory. We'll put together some docblocks and a documentation page in the future.

Let me know if you're having a specific problem where I can assist.

dfelton commented 5 years ago

Thank you @trowski. As you had suggested in the IRC chat room, the issue I was experiencing was able to be resolved with adjusting the frame size limit. Actually, both this and bytes per second limit.

I am curious... as, per your description above, the frame size setting can cause:

the connection is ended with a POLICY_VIOLATION

Therefore, it makes sense that my connection was being terminated. However, it wasn't until I adjusted both this and byes per second. Per above, bytes per second can cause:

being throttled.

Would it be your expectation that my app was terminating due to this (bytes per second)? The issue is resolved, so I ask this more for academic purposes at this point.

Lastly, I appreciate your time to lay out the above. This list could already prove to be a useful resource for others. If you like, I would be happy to submit another example. In it, provide a demo of connecting, using the Options object, and setting these settings (providing your comments above, in code comments for the readers of the example file).

$options = new Options();
$options = $options->withStreamThreshold(32768); //  number of bytes that will be buffered when streaming a message body before sending a frame.
// etc., etc.

While not an official "documentation" page for this, it gives some further visibility into what you've already answered above. Let me know if you are interested I'd be happy to do the writeup and submit a PR.

dfelton commented 5 years ago

My apologies, I completely did not realize until now, that I have created this Issue under the completely wrong repo (websocket-client vs websocket).

In any case, with your help and the issue resolved, I will close this issue out. I'll probably catch up with you on IRC concerning any contributions.

Thanks again.

dfelton commented 5 years ago

I believe this may be a correction to your comment above (~frame~ message)

Message size limit – maximum message size that can be received from the remote endpoint. If a larger ~frame~ message is received, the connection is ended with a POLICY_VIOLATION.

trowski commented 5 years ago

Would it be your expectation that my app was terminating due to this (bytes per second)? The issue is resolved, so I ask this more for academic purposes at this point.

No, that shouldn't happen, the connection should just be throttled. I think it's because the timer watcher in Rfc6455Client that resets the byte count for throttling is always unreferenced. That's fine in a server setting, but in a client that may be the only thing keeping the loop running. This commit will hopefully solve that problem.

I would be happy to submit another example

Sure, more examples are always welcome! If you want to contribute further to docs, we have a doc schema that is in other repos such as here that can be copied to this library. Those docs are automatically built and posted on amphp.org.

trowski commented 5 years ago

I pushed a commit that increases or removes some of the limits by default for the client. Does this seem reasonable? The defaults were geared more toward a server environment and aren't necessarily required for a client.