swift-nio icon indicating copy to clipboard operation
swift-nio copied to clipboard

MessageToByteHandler conformance to RemovableChannelHandler

Open AlexisQapa opened this issue 4 years ago • 5 comments

Expected behavior

MessageToByteHandler should conform to RemovableChannelHandler

Actual behavior

MessageToByteHandler doesn't conform to RemovableChannelHandler

Is there a reason for MessageToByteHandler to not conform to RemovableChannelHandler ?

SwiftNIO version/commit hash

SwiftNIO 2.34.0

Swift & OS version (output of swift --version && uname -a)

swift-driver version: 1.26.9 Apple Swift version 5.5.1 (swiftlang-1300.0.31.4 clang-1300.0.29.6)

AlexisQapa avatar Nov 23 '21 15:11 AlexisQapa

I agree, it should! There's no reason that support hasn't been added, it just wasn't high up on anyone's priority list. We'd happily accept a patch that adds the conformance.

Lukasa avatar Nov 23 '21 15:11 Lukasa

I'll try do add it then. Is there any gotcha I should be aware of (I don't now much about nio) ? ByteToMessagehander conformance seams quite complicated but some other conformances I read don't do anything.

AlexisQapa avatar Nov 23 '21 15:11 AlexisQapa

I don't think there is any particular gotcha except that we should allow implementers to intercept removal and do something with it.

Lukasa avatar Nov 23 '21 16:11 Lukasa

Implementers ? MessageToByteHandler is a final class. Do you mean to send event to decoders ? ByteToMessageHandler doesn't seem to notify it's encoder while being removed.

AlexisQapa avatar Nov 23 '21 16:11 AlexisQapa

ByteToMessageHandler does notify. Here's the conformance:

https://github.com/apple/swift-nio/blob/a58c36c55ce7b244a1b5dcec8c451ab06df3dc8c/Sources/NIOCore/Codec.swift#L720-L741

Note that this calls processLeftovers:

https://github.com/apple/swift-nio/blob/a58c36c55ce7b244a1b5dcec8c451ab06df3dc8c/Sources/NIOCore/Codec.swift#L543-L560

This calls decodeLoop with decodeMode set to .last, which will call decodeLast on the ByteToMessageDecoder:

https://github.com/apple/swift-nio/blob/a58c36c55ce7b244a1b5dcec8c451ab06df3dc8c/Sources/NIOCore/Codec.swift#L569-L603

We need a similar pattern: we need a new method on MessageToByteEncoder that MessageToByteHandler will call when it is removed from the pipeline.

Lukasa avatar Nov 23 '21 16:11 Lukasa