Skip to content

Make max message size configurable#57

Merged
trestletech merged 9 commits intomasterfrom
jeff/feature/maxmessagesize
Aug 5, 2019
Merged

Make max message size configurable#57
trestletech merged 9 commits intomasterfrom
jeff/feature/maxmessagesize

Conversation

@trestletech
Copy link
Contributor

@trestletech trestletech commented Jul 25, 2019

Closes #54

> ws <- WebSocket$new("ws://echo.websocket.org", maxMessageSize=3456)

Testing Notes

1. Try out really small maximums

This is intended to be run interactively! Running in a script will likely try to send before the connection is actually open.

ws2 <- websocket::WebSocket$new("ws://echo.websocket.org",
                                autoConnect = FALSE,
                                maxMessageSize=2)

ws2$onMessage(function(event) {
  message("Message received: ", event$data);
})
ws2$connect()
# MANUALLY WAIT
ws2$send(paste(rep("A", times=4), collapse=""))
ws2$close()

When you try to send/receive the message AAAA, you should get an error because the maximum size is 2 bytes.

2. Try out really large maximums

Start up a local websocket server so you can send really large messages successfully: https://github.com/rstudio/websocket/blob/master/tmp/websocketServer.R This will start a server on port 8080.

This is intended to be run interactively! Running in a script will likely try to send before the connection is actually open.

msgs <- NULL

ws2 <- websocket::WebSocket$new("ws://localhost:8080",
                                autoConnect = FALSE,
#                                maxMessageSize = 32 * 1024 * 1024) #32MB buffer (default)
#                                maxMessageSize = 50 * 1024 * 1024) #50MB buffer

ws2$onMessage(function(event) {
  msgs <<- c(msgs, event$data)
})
ws2$connect()
# MANUALLY WAIT
ws2$send(paste(rep("A", times=40000000), collapse="")) #Try sending a 40MB message
ws2$close()

When you try to send 40 million As, With the default maximum of 32MB, it should fail because it's too big. Once you up the max size to 50MB, you should then be able to send and receive such large messages. (This use case more accurately resembles the actual motivation for this work.)

@trestletech trestletech requested a review from alandipert July 25, 2019 19:16
@alandipert
Copy link
Contributor

Code looks ✨ , thank you. Would you please also update testing notes in the PR and add a bullet to NEWS.md? I bumped the version on master to 1.1.0.9001 and made a heading under which you may add said bullet.

Thanks! 🙏

@trestletech trestletech merged commit 34d28ff into master Aug 5, 2019
@trestletech trestletech deleted the jeff/feature/maxmessagesize branch August 5, 2019 14:14
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.

Allow changing max_message_size (frame size)

3 participants