Netty 4.1.23+ breaking changes against Netty 4.1.16 breaks Finagle

[vsabella]

Netty 4.1.23 introduced a breaking change that makes it binary incompatible with Netty 4.1.16.

Expected behavior

No exceptions when using Finagle inside of an environment with Netty 4.1.23 (such as Play Framework 2.6.13)

Actual behavior

When establishing a finagle connection, the following exception occurs.

java.lang.NoSuchMethodError: io.netty.channel.AbstractCoalescingBufferQueue.(I)V
at io.netty.handler.ssl.SslHandler$SslHandlerCoalescingBufferQueue.(SslHandler.java:1802)
at io.netty.handler.ssl.SslHandler.(SslHandler.java:356)
at io.netty.handler.ssl.SslHandler.(SslHandler.java:414)
at io.netty.handler.ssl.SslHandler.(SslHandler.java:398)
at io.netty.handler.ssl.SslHandler.(SslHandler.java:386)
at com.twitter.finagle.netty4.ssl.client.Netty4ClientSslChannelInitializer.createSslHandler(Netty4ClientSslChannelInitializer.scala:77)

Steps to reproduce the behavior

Force a maven dependency override to Netty 4.1.23 (or potentially greater).
Attempt to open an SSL connection via finagle-thrift.
This potentially also breaks in finagle-http2.

[vsabella]

We recently saw this issue when using Finagle clients with Play Framework 2.6.13 - or any one of our other services that forced an upgrade to Netty 4.1.23 but is calling other Finagle services via finagle-thrift .

I don't mind submitting a patch to Finagle to upgrade to latest Netty, but to avoid similar problems in the future would the group here find it unreasonable to shadow-jar Netty inside of Finagle.

(TODO: List pros/cons of shadow-jar)

[cacoco]

@vsabella in general Finagle is highly sensitive to a specific version of Netty -- the one listed in the build.sbt and is not expected to work with any other version. Updating Netty is generally a large process and not something we would likely accept in a pull request (sorry) as we have to move/fix internal things on occasion. With that, we are actively working to update to Netty 4.1.26 soon. Stay tuned, thanks!

[kevinoliver]

An update — we are working on moving to 4.1.28. Don't want to jinx it, but the early tests are looking good.

[spockz]

Shading netty in Finagle would be beneficial because it allows Finagle to stay on a specific netty version and have the rest of the application use another version of netty.

[cacoco]

@spockz Finagle originally bled many Netty types through its API and we are still paying the price for that. We are not quite done fully removing Netty types from all Finagle APIs. But that is the eventual goal which would allow for shading. Thanks.

[ryanoneill]

Closing this issue as Finagle has been on version 4.1.28 for a bit.