Clarifying EmbeddedChannel#writeOneOutbound versus EmbeddedChannel#write[AndFlush]

[jchambers]

Hello!

I've been writing some unit tests with EmbeddedChannel and have come to realize that there are things about its design/intent that I do not understand. The main thing I'm trying to understand is how EmbeddedChannel#writeOneOutbound is different from EmbeddedChannel#write (or #writeAndFlush), and vice versa for inbound operations.

Here's the sequence of events that led to my confusion: first, I wrote a test that depends on writeOneInbound that worked as expected. Here's a greatly simplified version:

@Test
void testWriteOneInbound() {
  // Note: normally, there would be some handlers in the pipeline, but I've
  // left them out in the interest of brevity.
  final EmbeddedChannel embeddedChannel = new EmbeddedChannel();
  final String message = "This should pass through the embedded channel";

  assertDoesNotThrow(() -> embeddedChannel.writeOneInbound(message).sync());
  assertEquals(message, embeddedChannel.inboundMessages().poll());
  assertTrue(embeddedChannel.inboundMessages().isEmpty());
}

…but then I tried to write an analogous test for writeOneOutbound that fails:

@Test
void testWriteOneOutbound() {
  final EmbeddedChannel embeddedChannel = new EmbeddedChannel();
  final String message = "This should pass through the embedded channel";

  assertDoesNotThrow(() -> embeddedChannel.writeOneOutbound(message).sync());
  assertEquals(message, embeddedChannel.outboundMessages().poll());
  assertTrue(embeddedChannel.outboundMessages().isEmpty());
}

Specifically, it fails because we detect a potential deadlock when waiting for the ChannelFuture returned by writeOneOutbound:

Caused by: io.netty.util.concurrent.BlockingOperationException: DefaultChannelPromise@9d5509a(incomplete)
	at io.netty.util.concurrent.DefaultPromise.checkDeadLock(DefaultPromise.java:463)
	at io.netty.channel.DefaultChannelPromise.checkDeadLock(DefaultChannelPromise.java:159)
	at io.netty.util.concurrent.DefaultPromise.await(DefaultPromise.java:248)
	at io.netty.channel.DefaultChannelPromise.await(DefaultChannelPromise.java:131)
	at io.netty.channel.DefaultChannelPromise.await(DefaultChannelPromise.java:30)
	at io.netty.util.concurrent.DefaultPromise.sync(DefaultPromise.java:405)
	at io.netty.channel.DefaultChannelPromise.sync(DefaultChannelPromise.java:119)
	at io.netty.channel.DefaultChannelPromise.sync(DefaultChannelPromise.java:30)
        ...

…but if I do essentially the same thing with writeAndFlush, it works as expected:

@Test
void testWriteAndFlush() {
  final EmbeddedChannel embeddedChannel = new EmbeddedChannel();
  final String message = "This should pass through the embedded channel";

  assertDoesNotThrow(() -> embeddedChannel.writeAndFlush(message).sync());
  assertEquals(message, embeddedChannel.outboundMessages().poll());
  assertTrue(embeddedChannel.outboundMessages().isEmpty());
}

Now, I'm sure I can dig into the code and figure out what's different between these cases, but I'm less confident that I'll be able to understand the design intent. I haven't been able to find much in the docs or other discussion threads, so I figured I'd ask here. Is this expected behavior? If so, how are we supposed to use writeOneOutbound? When should we use that instead of EmbeddedChannel#write[AndFlush]?

Thanks kindly!

---

EDIT: I've poked a little more and I think I can refine this question a bit. The docs for EmbeddedChannel#writeOneOutbound say:

Writes one message to the outbound of this Channel and does not flush it. This method is conceptually equivalent to AbstractChannel.write(Object).

…and fair enough! If I do a conventional write without flushing, I get the same failure as I do with writeOneOutbound:

@Test
void testWrite() {
  final EmbeddedChannel embeddedChannel = new EmbeddedChannel();
  final String message = "This should pass through the embedded channel";

  assertDoesNotThrow(() -> embeddedChannel.write(message).sync());
  assertEquals(message, embeddedChannel.outboundMessages().poll());
  assertTrue(embeddedChannel.outboundMessages().isEmpty());
}

…but then that brings me back to the headline question: is there a difference between these sets of methods? When should callers prefer one approach over another?

[jchambers]

Now, I'm sure I can dig into the code and figure out what's different between these cases, but I'm less confident that I'll be able to understand the design intent.

I really should have just dug into the source code 🤦‍♂️

The intent is clear: the writeOne{Inbound|Outbound} methods:

• Check that the channel is open, recording an exception if not
• Perform the read/write if the channel is open
• Mark the returned future failed with any previously-recorded exception; otherwise, mark it as succeeded

I will open a pull request to note these behaviors in the docs.

That said: it does seem a little weird to me that the future returned by writeOneOutbound is all but guaranteed to fail on await/sync. If anybody has insight as to the design intent there, I'd love to document that, too!