LogbookServerHandler - attempt to write logs when the handler is prematurely removed from the ReactorNetty pipeline

[kasmarian]

Description

LogbookServerHandler's channelRead() method is expecting to be invoked at least once with message object that is instance of io.netty.handler.codec.http.LastHttpContent.

When the handler is added via reactor.netty.Connection.addHandlerLast() method, ReactorNetty registers the handler for removal on context termination, and this termination is sometimes triggered before the last LastHttpContent message is passed to the read method. Leaving the LogbookServerHandler's Sequence object only with response written.

This change proposes to add a trigger on handlerRemoved() method of io.netty.channel.ChannelHandlerAdapter to see if the response writing stage was already submitted to the Sequence and add the request writing stage and trigger the runnables for both stages.

Below is the initial thought process, which proved to be wrong. The issue turned out to be not in the order of the LogbookServerHandler (it's added in the same place eventually for the bare spring boot webflux app between HttpTrafficHandler and ReactiveBridge), but in the way it is added.

This happens when the handler is added using addHandlerLast(), which positioned it too late in the pipeline. By that point, other handlers (like content aggregators) had already consumed the LastHttpContent messages. When LogbookServerHandler was positioned incorrectly, it only saw the write() events but missed the complete read() events.

This change proposes to position the LogbookServerHandler in the pipeline at the point when the message is readable and bufferable, but before handlers that may skip lead to skipping the pipeline calls for the last closing message.

The solution also offers the following order of checks as none of the netty handlers that the solution relies on are mandatory (see NettyPipeline):

First priority: After HttpTrafficHandler (where HTTP messages are fully decoded but not yet consumed)
Second priority: After HttpCodec (when HttpTrafficHandler isn't available)
Last resort: At the end of the pipeline (as it works now)

Motivation and Context

Relates to #2053, #1555, #1926

Types of changes

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

• My change requires a change to the documentation.
• I have updated the documentation accordingly.
• I have added tests to cover my changes.
• All commits are signed

[ChristianLohmann]

👍

[kasmarian]

👍