madari / go-socket.io

A Socket.IO backend implementation written in Go
http://gopkgdoc.appspot.com/pkg/github.com/madari/go-socket.io
MIT License
412 stars 64 forks source link

connection.go: reader() - cannot send messages over 4096 bytes #7

Closed justinfx closed 13 years ago

justinfx commented 13 years ago

It seems that even though you can set c.sio.config.ReadBufferSize to any size, the underlying transport buffers are still using the default 4096 buffer value? So as it stands, if you increase ReadBufferSize beyond 4096 and send a large message, you get a bufio: Buffer Full, which is unhandled and drops the connection. I have been trying to work out a temporary solution where it checks for this error, and continues to loop and build the total amount until EOF. But mainly its been a hack.

Is this something that is going to be solved in the streaming codec with v0.7? Or can you see any clear solutions as it stands? I was able to get my hack to work in the sense of allowing an unlimited size message, but obviously that is not desirable, and the value of ReadBufferSize should really set a true limit. Maybe there needs to be a MaxMessageSize or something, so the ReadBufferSize can keep being reused to build up the total message UP TO MaxMessageSize. If it exceeds that, it would send back some kind of socketio failure, or, i guess drop the connection still.

madari commented 13 years ago

Hi! You have been really active lately! Thank you for that. This is caused by a known bug in the websocket-package: http://code.google.com/p/go/issues/detail?id=2134

Other transports work fine.

As it stands, go-socket.io does not limit the maximum size of messages passed through. The ReadBufferSize configuration option can be used to fine-tune performance (bigger buffer = less syscalls) in special cases.

We could probably also add a mechanism to optionally limit the maximum size of messages to some value later.

madari commented 13 years ago

The new websocket package was submitted to the Go tip yesterday. It fixes this issue.