Ping frames are not decoded anymore and close the socket

[PhilLehmann]

Version

• vert.x core: 3.5.0
• vert.x tcp eventbus bridge: 3.5.0

Context

I am currently working von vertx-eventbus-bridge-client.

On my current branch (https://github.com/philrykoff/vertx-eventbus-bridge-clients/tree/java-client; PR: https://github.com/vert-x3/vertx-eventbus-bridge-clients/pull/18), which was just upgraded to 3.5.0, sending a message {"type":"ping"} to the bridge causes the bridge to log

Okt 27, 2017 11:15:28 AM io.vertx.ext.eventbus.bridge.tcp.impl.TcpEventBusBridgeImpl
SCHWERWIEGEND: msg does not have address: {"type":"ping"}

and return an error to the client.

Do you have a reproducer?

Test TcpBusTest.testPing in https://github.com/philrykoff/vertx-eventbus-bridge-clients/tree/java-client

Steps to reproduce

1. Check out
2. Run Test TcpBusTest.testPing
3. See console

Extra

It looks like the checkCallHook block which was inserted before discarding the ping message should be placed after discarding ping messages. Also, there are two checks for address == null, once inside the lambda, once below discarding the ping. Maybe one is enough?

https://github.com/vert-x3/vertx-tcp-eventbus-bridge/blob/3.5.0/src/main/java/io/vertx/ext/eventbus/bridge/tcp/impl/TcpEventBusBridgeImpl.java#L253-L273

[sandipsahoo2k2]

Just now I upgraded my server from 3.4.1 to 3.5.0 and I started getting exactly same error my TCP Event bus started failing and my android app is not working any more.. I am getting IllegalArgumentException No Enum Constant PING and socket closed.

[sandipsahoo2k2]

I found that TCP Event bus was never sending CREATE, CLOSE neither PING Events too with the above exception I have fixed all and published it. Couldn't push my changes to this branch/pull request hence I updated my branch with the fixes https://github.com/sandipsahoo2k2/vertx-tcp-event-bridge

[sandipsahoo2k2]

There is also one more problem now in the data in eventbus "writehandler" or the socketID as part of the data is missing ! So its a big breaking change for projects who have relied on 3.4.1 I am facing the heat :(

[QingMings]

same error info

[vietj]

@sandipsahoo2k2 can you provide reproducers for the regressions ?

[vietj]

@philrykoff I fixed this in master, can you check it's fine on your side ?

[vietj]

@sandipsahoo2k2 can you open new issues for the issues you mentionned and possibly provide a fix / reproducer ?

[PhilLehmann]

Sorry for taking so long ☹️

There is no error being logged anymore server side.

But the client now receives a response message { "type": "err", "message": "unknown_type" }.

[vietj]

for which message ?

[PhilLehmann]

For each ping.

I stopped during debug, so that multiple pings would be send and for each, one unknown_type error would be send by the server.

[vietj]

with 3.5.1-SNAPSHOT ?

[PhilLehmann]

Yes.

It seems, doSendOrPub is being executed for PINGs, too:

https://github.com/vert-x3/vertx-tcp-eventbus-bridge/blob/70f1ee88558f8601ce2b08bb8dfb1a871382987a/src/main/java/io/vertx/ext/eventbus/bridge/tcp/impl/TcpEventBusBridgeImpl.java#L263

It looks like we return for address == null, but not for pings.

Again, sorry for the late reply...

[vietj]

would you mind to provide a patch for the bridge ?

I've added a fix and test here https://github.com/vert-x3/vertx-tcp-eventbus-bridge/commit/70f1ee88558f8601ce2b08bb8dfb1a871382987a but it does not seem enough

[PhilLehmann]

No problem. Commits don't have to be signed for that one, right?

[vietj]

no they don't but we need CLA sign though

[PhilLehmann]

Done & tested: https://github.com/vert-x3/vertx-tcp-eventbus-bridge/pull/33.

Thanks a lot!