Skip to content
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

WebSocket does not handle fragmentation or messages >1 MB, disconnects silently #24

Closed
mafredri opened this issue Jun 16, 2017 · 6 comments

Comments

@mafredri
Copy link

The WebSocket implementaiton for the Chrome DevTools does not seem to handle either fragmentation (continuation frames) or messages that are larger than 1 MB in size.

This problem is reproducible after connecting to the webSocketDebuggerUrl of a page and either sending fragmented messages or messages above 1 MB in size.

In both cases the result is silent failure of the tcp connection. The error is found upon next write when the connection returns (among other things)write tcp 127.0.0.1:50872->127.0.0.1:9222: write: broken pipe.

I spent some time tracking down these two issues and came to the following conclusions:

  1. Chrome does not support websocket fragmentation (continuation frames), they remain unimplemented at the time of writing (net/server/web_socket_encoder.cc#86)

  2. The underlying HTTP connection does not support bigger payloads than 1MB, I believe the reason is the kDefaultMaxBufferSize, defined here: net/server/http_connection.h#33

    When we send messages larger than 1MB, Chrome logs the following:

    [89253:55299:0616/194024.005586:ERROR:http_connection.cc(37)] Too large read data is pending: capacity=1048576, max_buffer_size=1048576, read=1048576
    

The biggest use-case for this is probably sending large scripts over CDP, however, the current behavior does lead to hard-to-diagnose bugs. It would be nice if CDP could support WebSocket fragmentation, and at the very least provide useful error messages when it cannot handle a command.

Related issues:

@matthewmueller
Copy link

matthewmueller commented Jun 28, 2017

I've also noticed that if you're trying to capture largeish screenshots (format: png, quality: 100, height: 950, width: 1920), you'll get abrupt disconnections.

I believe it's for the same reason, though going the other way: client <= chrome target

@mafredri
Copy link
Author

mafredri commented Jul 1, 2017

@matthewmueller I'm not able to reproduce your issue on either Chrome Beta (60.0.3112.50) or Canary (61.0.3146.0) on macOS. I tested both headless and non-headless with very large window sizes, producing screenshots at 16000x4000 and ~20mb.

Tested with my Golang lib (cdp) and the Page.captureScreenshot-method (cdp.Page.CaptureScreenshot).

@pavelfeldman
Copy link
Contributor

@mafredri thanks for the report.

we haven't had a client that would send fragmented messages, it surely can be implemented. What is your client? similarly, turns out we've never seen large incoming messages and I can totally see how injecting large scripts could require those.

Since those are outside of the scope of the remote debugging protocol, but are rather chrome bugs, I filed it in a chrome repo.

@mafredri
Copy link
Author

mafredri commented Jul 5, 2017

Thanks for the response @pavelfeldman!

The client I'm using is gorilla/websocket for Golang which sends fragmented messages after its buffer size is exceeded. As noted though, even without fragmentation, messages >1mb can't be sent. It would be very nice if the protocol supported this 😄! It could allow us to work around the 1mb limitation as well.

Big thanks for filing the issue, I was actually unsure where to file it. I guessed the chromium repo was the best place but I've never filed an issue there before 😅.

@pavelfeldman
Copy link
Contributor

I'm bumping it to a 100MB here - can't do anything smart here since it needs to stay non-blocking.

@matthewmueller
Copy link

that's wonderful news @pavelfeldman!

sorry late follow-up here, the large screenshots turned out to be a bug in my code. sorry for the false alarm and thanks for checking it out @mafredri!

mafredri added a commit to mafredri/cdp that referenced this issue Jul 25, 2017
The 1MB message size limit was recently bumped to 100MB in Chrome, this
means there is no reason to enforce the 1MB limit any longer.

See: ChromeDevTools/devtools-protocol#24 and
https://chromium-review.googlesource.com/c/560777/

This limitation was in place to improve the user experience (guard
against abrupt disconnection). However, the user is still guarded by the
default buffer size of 4096, writes larger than this will return an
error asking the user to raise the buffer size. These limitations have
now been documented in WithWriteBufferSize so that they can easily be
discovered.
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

No branches or pull requests

4 participants