Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to configure Netty http compression #4056

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ArtyomGabeev
Copy link

Fixes: #2878

I'm unsure if I need to add tests since Netty provides the HttpContentCompressor.

Another question: I do not know about HttpStreamsServerHandler from Play Framework. Is it used for chunked HTTP responses?

HTTP compression should work with chunked responses by compressing each chunk. I need to check if the order of handlers is correct.

Thanks,
Artyom

@ArtyomGabeev
Copy link
Author

ArtyomGabeev commented Sep 22, 2024

Manually checked with ZIO netty server, compression works fine with ZIOStream's.

@adamw
Copy link
Member

adamw commented Sep 23, 2024

Thanks! HttpStreamsServerHandler is used for web sockets, so shouldn't (ideally ;) ) matter for these changes. But anyway, we do need tests - verifying if the compressor indeed is used (that is, if you indeed receive compressed responses), if there are no interactions with WS, and if it's in the right place. Not sure right now how it should be position wrt to handler and Logging, for example

@ArtyomGabeev
Copy link
Author

Ill check the LoggingHandler, but so far it works fine for me.
Ill check existing tests to find appropriate place for compression tests.

@adamw
Copy link
Member

adamw commented Sep 24, 2024

Maybe in NettyFutureServerTests, as these tests are netty-specific? https://github.com/softwaremill/tapir/blob/master/server/netty-server/src/test/scala/sttp/tapir/server/netty/NettyFutureServerTest.scala#L17

You can see an example on how to add a custom test e.g. here: https://github.com/softwaremill/tapir/blob/master/server/netty-server/sync/src/test/scala/sttp/tapir/server/netty/sync/NettySyncServerTest.scala#L45

)

def defaultInitPipeline(cfg: NettyConfig)(pipeline: ChannelPipeline, handler: ChannelHandler): Unit = {
cfg.sslContext.foreach(s => pipeline.addLast(s.newHandler(pipeline.channel().alloc())))
pipeline.addLast(ServerCodecHandlerName, new HttpServerCodec())
if (cfg.compressionEnabled) pipeline.addLast(new HttpContentCompressor(cfg.compressionContentSizeThreshold, Seq.empty: _*))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This way it was failing for me in Runtime for netty 4.1.112 and Tapir 1.10.15

[info] 2024-11-23 21:07:49.260 WARN  io.netty.channel.ChannelInitializer - Failed to initialize a channel. Closing: [id: 0xf0ede386, L:/127.0.0.1:9000 - R:/127.0.0.1:59802]
[info] java.lang.ClassCastException: class [Ljava.lang.Object; cannot be cast to class [Lio.netty.handler.codec.compression.CompressionOptions; ([Ljava.lang.Object; is in module java.base of loader 'bootstrap'; [Lio.netty.handler.codec.compression.CompressionOptions; is in unnamed module of loader 'app')
[info] 	at server.HttpServer$.pipeline$1(HttpServer.scala:42)
[info] 	at server.HttpServer$.$anonfun$run$4(HttpServer.scala:50)
[info] 	at server.HttpServer$.$anonfun$run$4$adapted(HttpServer.scala:50)
[info] 	at sttp.tapir.server.netty.internal.NettyBootstrap$$anon$1.initChannel(NettyBootstrap.scala:26)
[info] 	at io.netty.channel.ChannelInitializer.initChannel(ChannelInitializer.java:129)
[info] 	at io.netty.channel.ChannelInitializer.handlerAdded(ChannelInitializer.java:112)
[info] 	at io.netty.channel.AbstractChannelHandlerContext.callHandlerAdded(AbstractChannelHandlerContext.java:1130)
[info] 	at io.netty.channel.DefaultChannelPipeline.callHandlerAdded0(DefaultChannelPipeline.java:608)
[info] 	at io.netty.channel.DefaultChannelPipeline.access$100(DefaultChannelPipeline.java:45)
[info] 	at io.netty.channel.DefaultChannelPipeline$PendingHandlerAddedTask.execute(DefaultChannelPipeline.java:1460)
[info] 	at io.netty.channel.DefaultChannelPipeline.callHandlerAddedForAllHandlers(DefaultChannelPipeline.java:1114)
[info] 	at io.netty.channel.DefaultChannelPipeline.invokeHandlerAddedIfNeeded(DefaultChannelPipeline.java:649)
[info] 	at io.netty.channel.AbstractChannel$AbstractUnsafe.register0(AbstractChannel.java:513)
[info] 	at io.netty.channel.AbstractChannel$AbstractUnsafe.access$200(AbstractChannel.java:428)
[info] 	at io.netty.channel.AbstractChannel$AbstractUnsafe$1.run(AbstractChannel.java:485)
[info] 	at io.netty.util.concurrent.AbstractEventExecutor.runTask(AbstractEventExecutor.java:173)
[info] 	at io.netty.util.concurrent.AbstractEventExecutor.safeExecute(AbstractEventExecutor.java:166)
[info] 	at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:469)
[info] 	at io.netty.channel.epoll.EpollEventLoop.run(EpollEventLoop.java:408)
[info] 	at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:994)
[info] 	at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74)
[info] 	at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30)
[info] 	at java.base/java.lang.Thread.run(Thread.java:1575)

The following code works

          new HttpContentCompressor(config.compressionThreshold, List.empty[CompressionOptions]: _*)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Add out-of-the-box compression support to Tapir
3 participants