Unregistering a consumer fails

PhilLehmann commented 6 years ago

Version

vert.x core: 3.5.1-SNAPSHOT
vert.x tcp eventbus bridge: 3.5.1-SNAPSHOT

Context

While unregistering a consumer, vertx-eventbus-bridge-clients fails:

Unhandled exception java.lang.IllegalArgumentException: Invalid frame type unregister at io.vertx.ext.eventbus.bridge.tcp.impl.TcpEventBusBridgeImpl.parseType(TcpEventBusBridgeImpl.java:409) at io.vertx.ext.eventbus.bridge.tcp.impl.TcpEventBusBridgeImpl.lambda$handler$8(TcpEventBusBridgeImpl.java:252) at io.vertx.ext.eventbus.bridge.tcp.impl.protocol.FrameParser.handle(FrameParser.java:67) at io.vertx.ext.eventbus.bridge.tcp.impl.protocol.FrameParser.handle(FrameParser.java:30) at io.vertx.core.net.impl.NetSocketImpl$DataMessageHandler.handle(NetSocketImpl.java:384) at io.vertx.core.net.impl.NetSocketImpl.handleMessageReceived(NetSocketImpl.java:351) at io.vertx.core.net.impl.NetServerImpl$2.handleMessage(NetServerImpl.java:446) at io.vertx.core.net.impl.NetServerImpl$2.handleMessage(NetServerImpl.java:443) at io.vertx.core.net.impl.VertxHandler.lambda$channelRead$1(VertxHandler.java:146) at io.vertx.core.impl.ContextImpl.lambda$wrapTask$2(ContextImpl.java:337) at io.vertx.core.impl.ContextImpl.executeFromIO(ContextImpl.java:195) at io.vertx.core.net.impl.VertxHandler.channelRead(VertxHandler.java:144) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:340) at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1359) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:362) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:348) at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:935) at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:134) at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:645) at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:580) at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:497) at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:459) at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:858) at java.lang.Thread.run(Thread.java:745)

Extra

In 3.4.2, a PING message was detected with

https://github.com/philrykoff/vertx-tcp-eventbus-bridge/blob/3.4.2/src/main/java/io/vertx/ext/eventbus/bridge/tcp/impl/TcpEventBusBridgeImpl.java

      final String type = msg.getString("type", "message");

      if ("ping".equals(type)) {
        // discard
        return;
      }

In the current version, the method parseType has been added for detecting PING messages. But it currently only allows for four message types:

  private static BridgeEventType parseType(String typeStr) {
    switch (typeStr) {
      case "ping":
        return BridgeEventType.SOCKET_PING;
      case "register":
        return BridgeEventType.REGISTER;
      case "publish":
        return BridgeEventType.PUBLISH;
      case "send":
        return BridgeEventType.SEND;
      default:
        throw new IllegalArgumentException("Invalid frame type " + typeStr);
    }
  }

We have two options:

extending method parseType or
falling back to the easier string comparison performed before.

I would make this dependant on whether you would like to use parseType anywhere else in this project. If it will just be used for pings, I would fall back.

Opinions?