-
Notifications
You must be signed in to change notification settings - Fork 253
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
Add a compat flag for increasing the WS message size limit #2164
base: main
Are you sure you want to change the base?
Conversation
5c85dd3
to
25ef42e
Compare
increaseWebsocketMessageSize @50 :Bool | ||
$compatEnableFlag("increase_websocket_message_size") | ||
$experimental; | ||
# Experimental, increase the WebSocket message size limit to 128MB |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In order to avoid misleading people about the intention here, can we please say:
For local development purposes only, increase the message size limit to 128MB. This is not expected ever to be made available in production, as large messages are inefficient.
@@ -425,6 +425,9 @@ class WebSocket: public EventTarget { | |||
// `close()`, thereby preventing calls to `send()` even after we wake from hibernation. | |||
bool closedOutgoingForHib = false; | |||
|
|||
// Maximum allowed size for WebSocket messages | |||
size_t maxMessageSize = 1u << 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use the constant kj::WebSocket::SUGGESTED_MAX_MESSAGE_SIZE
.
@@ -425,6 +425,9 @@ class WebSocket: public EventTarget { | |||
// `close()`, thereby preventing calls to `send()` even after we wake from hibernation. | |||
bool closedOutgoingForHib = false; | |||
|
|||
// Maximum allowed size for WebSocket messages | |||
size_t maxMessageSize = 1u << 20; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Instead of having this be a member of WebSocket
, could you pass it as a parameter to readLoop()
(reading the feature flag in startReadLoop()
)?
This is one step towards addressing #2051 (Wrangler changes will also be needed)