mrniko / netty-socketio

Socket.IO server implemented on Java. Realtime java framework
Apache License 2.0
6.82k stars 1.65k forks source link

Binary / byte array messages broken. #191

Closed jimmyryan15 closed 9 years ago

jimmyryan15 commented 9 years ago

Thanks for this awesome framework. I was excited to add the binary support to our product. The problem is I can't get the basic byte[] pushed down based on the demo.

I'm using 1.7.5 (and even tried snapshot). The BinaryEventLauncher and corresponding binary-event-index.html page.

1) The client on 'msg' doesn't get invoked unless I use Byte[](instead of the demo code that has byte[]) on the server side. I wouldn't think this matters but for some reason it does. SERVER SIDE BELOW

2) Even when the client gets called back it throws an "Uncaught RangeError: Invalid typed array length" exception when constructing the Uint8Array(file). CLIENT SIDE BELOW

SERVER SIDE server.addEventListener("msg", Byte[].class, new DataListener<Byte[]>() { @Override public void onData(SocketIOClient client, Byte[] data, AckRequest ackRequest) {

            //client.sendEvent("msg", data);
            ((ClientOperations) client).sendEvent("msg", data);
        }
    });

CLIENT SIDE (Copied from demo) socket.on('msg', function(file) {

                  var arrayBuffer = new Uint8Array(file).buffer;
                  var blob        = new Blob([arrayBuffer]);

                  var imgList = $('ul#img-list');

                  var li = $('<li/>').appendTo(imgList);
                  $('<div/>').text(file.name).appendTo(li);
                  var img = $('<img/>').appendTo(li);

                  var reader = new FileReader();
                  reader.onload = (function(aImg) {
                    return function(e) {
                      aImg.attr('src', e.target.result);
                      aImg.attr('width', 150);
                    };
                  })(img);

                  reader.readAsDataURL(blob);
    });
jimmyryan15 commented 9 years ago

Update. The callback handle in the on 'msg' isn't a blob / file. when I deref the .buffer var it is "undefined". I have tried sending Packet with subType = BINARY_EVENT and for some reason my chrome browser spins out of control until it crashes.

Was this perhaps fixed as in the discussion at Binary Support, sending binary to client #178 but somehow not rolled forward to the latest?

Thanks

mrniko commented 9 years ago

try to use byte[] not Byte[]

jimmyryan15 commented 9 years ago

The default code in the example uses primitive byte array and doesn't end up generating a client side callback. Also I noticed that the client side code in the binary demo has the following in the msg handler:

new Uint8Array(file).buffer

That can't be right right? It should be (file.buffer) Either way in no scenario does the client callback happen.

Can you try the checked in demo against snapshot and advise.

Thanks

mrniko commented 9 years ago

You mean binary-event example is not working at all?

jimmyryan15 commented 9 years ago

Correct. Using both the latest release as well wells snapshot. There is definitely something not right on the server side. However I don't see how the client could work either given what I mentioned about calling .buffer on the Uint8Array.

mrniko commented 9 years ago

Yes, it's a bug, but only for websocket transport. For longpolling it works.

mrniko commented 9 years ago

it works for websocket too! but you should add config.setMaxFramePayloadLength(200 * 1024); if you load big images

jimmyryan15 commented 9 years ago

Are you saying that websocket works if you make make that setting? I I can't get even dummy simple byte arrays to go. Else are there plans to support websocket? Thanks

mrniko commented 9 years ago

The correct behavior is added image on the screen after you push 'browse' button and select image. Do you see this?

jimmyryan15 commented 9 years ago

No

mrniko commented 9 years ago

Which browser you used? it was tested with latest firefox and chrome

jimmyryan15 commented 9 years ago

Chrome 39 and FF 29. Same thing, client on 'msg' doesn't get invoked.

mrniko commented 9 years ago

i updated socketio.js lib in netty-socketio-demo maybe it's a root of the problem

mrniko commented 9 years ago

You can you "turn on" the logs on the server by switching log level for "com.corundumstudio" package to "TRACE" and check does server sends packet or not

jimmyryan15 commented 9 years ago

Thoughts?

[2015-01-12 09:20:36,100] [TRACE] [handler.InPacketHandler] [nioEventLoopGroup-3-1] In message: 451-["msg",{"_placeholder":true,"num":0}] sessionId: 6656ec2a-b31c-491b-8624-57288578938e [2015-01-12 09:20:36,101] [DEBUG] [websocketx.WebSocket08FrameDecoder] [nioEventLoopGroup-3-1] Decoding WebSocket Frame opCode=2 [2015-01-12 09:20:36,101] [DEBUG] [websocketx.WebSocket08FrameDecoder] [nioEventLoopGroup-3-1] Decoding WebSocket Frame length=1060 [2015-01-12 09:20:36,104] [TRACE] [handler.InPacketHandler] [nioEventLoopGroup-3-1] In message: ?PNG

[RAW PNG BYTES HERE] [2015-01-12 09:20:43,305] [TRACE] [handler.EncoderHandler] [nioEventLoopGroup-3-1] Out message: 451-["msg",{"num":0,"_placeholder":true}] sessionId: 6656ec2a-b31c-491b-8624-57288578938e [2015-01-12 09:20:43,305] [DEBUG] [websocketx.WebSocket08FrameEncoder] [nioEventLoopGroup-3-1] Encoding WebSocket Frame opCode=1 length=41 [2015-01-12 09:20:43,306] [DEBUG] [websocketx.WebSocket08FrameDecoder] [nioEventLoopGroup-3-1] Decoding WebSocket Frame opCode=1 [2015-01-12 09:20:43,306] [DEBUG] [websocketx.WebSocket08FrameDecoder] [nioEventLoopGroup-3-1] Decoding WebSocket Frame length=1 [2015-01-12 09:20:43,306] [TRACE] [handler.InPacketHandler] [nioEventLoopGroup-3-1] In message: 2 sessionId: 6656ec2a-b31c-491b-8624-57288578938e [2015-01-12 09:20:43,307] [TRACE] [handler.EncoderHandler] [nioEventLoopGroup-3-1] Out message: 3 sessionId: 6656ec2a-b31c-491b-8624-57288578938e [2015-01-12 09:20:43,307] [DEBUG] [websocketx.WebSocket08FrameEncoder] [nioEventLoopGroup-3-1] Encoding WebSocket Frame opCode=1 length=1

mrniko commented 9 years ago

This issue has been introduced since netty-4.0.24 version. I make some changes and update it to latest version. Now all works fine!

jimmyryan15 commented 9 years ago

Good to know. I just saw that PacketEncoder.encodePacket in the finally block looks like the following: // we need to write a buffer in any case if (!binary) { buffer.writeByte(0); int length = buf.writerIndex(); buffer.writeBytes(longToBytes(length)); buffer.writeByte(0xff); buffer.writeBytes(buf);

            buf.release();
        }

If i comment out the !binary if then the packet goes out. However the client file.buffer is empty

mrniko commented 9 years ago

no, you don't need to touch this code :)

mrniko commented 9 years ago

so, is it works now?

jimmyryan15 commented 9 years ago

I synced your change for #191 and I get the same result. In fact if the PacketEncode.encodePacket finally block doesn't have the change where I comment out the "if (!binary) {" if statement then the packet doesn't go out at all. If I comment that out I still don't a file.buffer that has anything on the client side.

mrniko commented 9 years ago

which netty version do you use now?

jimmyryan15 commented 9 years ago

netty-all-4.0.24.Final.jar This was built using the POM and thus corresponding dependent jars

jimmyryan15 commented 9 years ago

For the PacketEncoder, the comment in the finally block says "we need to write a buffer in any case". However if the argument "binary" is true then the buffer isn't written and released.

That being said even when I comment it out the file.buffer is non existent.

To summarize: 1) I have the latest server code (with #191 applied) 2) I have the updated socket.io for demo project 3) The demo code has bug in it re: new Uint8Array(file).buffer instead of new Uint8Array(file.buffer). Even when that is corrected the "file" ref doesn't have a buffer

Thanks for looking at this

mrniko commented 9 years ago

@jimmyryan15

I have changed var arrayBuffer = new Uint8Array(file).buffer; to your code var arrayBuffer = new Uint8Array(file.buffer); and my code stops to work too. Please revert this change and try again

jimmyryan15 commented 9 years ago

Interesting. If I i) Apply the #191 change you posted ii) I comment out the PacketEncoder.encodePacket finally block re: binary if iii) Use the "var arrayBuffer = new Uint8Array(file).buffer" line in the client then the image on the client DOES work!

I have 2 questions though: A) How does it work for you w/o ii) above? B) My chrome browser is showing the following errors (even though the image shows up now) which in turn disconnects the socket.io. Note: no exception is thrown on the server side.

POST http://192.168.1.3:9092/socket.io/?EIO=3&transport=polling&t=1421089896057-7&sid=d646898f-9780-4ed8-a07b-d42fd1575d2c 500 (Internal Server Error) POST http://192.168.1.3:9092/socket.io/?EIO=3&transport=polling&t=1421089896065-8&sid=d646898f-9780-4ed8-a07b-d42fd1575d2c 500 (Internal Server Error)

mrniko commented 9 years ago

500 (Internal Server Error)

Is there any server error in logs?

jimmyryan15 commented 9 years ago

There aren't any exceptions that I see in trace level logging (per my previous NOTE: no exception is thrown on the server side). That being said I don't doubt that the browser is seeing the 500's but they must be happening at some internal layer (perhaps inside netty).

I'm sorry but I can't spend any more time on this. I'm going to leave the messages as base64 encoded and handle as String because I know that it works. I spent 4 days troubleshooting this and obviously there are plumbing level issues and the binary example doesn't work w/o changing the PacketEncoder.encodePacket finally block for me and even then I'm now getting the 500 exceptions that break the websocket.

I don't understand how yours would possibly work w/o the PacketEncoder writing the bytes. The change you made is based on a breaking change in netty but your POM file imports that version of netty!

If you can get the main and demo project to a place where the binary event works out of the box then please update here and I'll try it again.

I think your project is great and clearly you are attentive but I must say that I get the sense you aren't testing on the exact same context avail to us in the repos.

jimmyryan15 commented 9 years ago

If you do figure out how to make this repeatably work please update the POM so that all dependencies are set to what you are using. I just realized that you said "This issue has been introduced since netty-4.0.24 version. I make some changes and update it to latest version. Now all works fine!"

Does this mean that we need the snapshot of netty? etc. etc.

It would be very clear if the project specifically declared what was needed.

Thanks

mrniko commented 9 years ago

I must say that I get the sense you aren't testing on the exact same context avail to us in the repos.

Sorry for that. I tested demo with netty-4.0.23 in classpath

Here is my netty-socketio-demo project. https://yadi.sk/d/jBhpOBAddw9Dy

  1. build latest netty-socketio 1.7.6-SNAPSHOT
  2. build my netty-socketio-demo
  3. run mvn exec:java -Dmain.class=com.corundumstudio.socketio.demo.BinaryEventLauncher
  4. open the netty-socketio-demo\client\binary-event-index.html page in your browser
jimmyryan15 commented 9 years ago

Now we are talking. I'll try this in the morning and see what we get. Thanks

mrniko commented 9 years ago

have you try it?