Repository: netty/netty
Issue: codec-http2: Http2FrameCodec never satisfies promise on Http2WindowUpdateFrame

I noticed that when Http2WindowUpdateFrame is written to Http2FrameCodec, the write [promise is never satisfied](https://github.com/netty/netty/blob/4.1/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java#L144).  Is this intentional?  If not, I'm happy to submit a fix.


Issue comments:
@olix0r nope... A PR would be awesome :)

@olix0r actually @Scottmitch made a good point. Can you wrap the code into a try / catch block and fail the promise on error ?


Pull request review comments:
@buchgr should we remove the `exceptionCaught(...)` here ? I think if we propagate to the promise we should not also call `exceptionCaught(...)`. Also @nmittler @Scottmitch WDYT ?


Patch that resolved the issue:
diff --git a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java
index 23769cfc9a9..b9cd604063a 100644
--- a/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java
+++ b/codec-http2/src/main/java/io/netty/handler/codec/http2/Http2FrameCodec.java
@@ -141,8 +141,7 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
         try {
             if (msg instanceof Http2WindowUpdateFrame) {
                 Http2WindowUpdateFrame frame = (Http2WindowUpdateFrame) msg;
-                consumeBytes(frame.streamId(), frame.windowSizeIncrement());
-                promise.setSuccess();
+                consumeBytes(frame.streamId(), frame.windowSizeIncrement(), promise);
             } else if (msg instanceof Http2StreamFrame) {
                 writeStreamFrame((Http2StreamFrame) msg, promise);
             } else if (msg instanceof Http2GoAwayFrame) {
@@ -155,13 +154,14 @@ public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise promise)
         }
     }
 
-    private void consumeBytes(int streamId, int bytes) {
+    private void consumeBytes(int streamId, int bytes, ChannelPromise promise) {
         try {
             Http2Stream stream = http2Handler.connection().stream(streamId);
             http2Handler.connection().local().flowController()
                         .consumeBytes(stream, bytes);
+            promise.setSuccess();
         } catch (Throwable t) {
-            exceptionCaught(ctx, t);
+            promise.setFailure(t);
         }
     }
 


Test patch:
diff --git a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java
index e404e396f08..c8fabd3004e 100644
--- a/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java
+++ b/codec-http2/src/test/java/io/netty/handler/codec/http2/Http2FrameCodecTest.java
@@ -402,9 +402,24 @@ public void windowUpdateFrameDecrementsConsumedBytes() throws Exception {
         frameListener.onDataRead(http2HandlerCtx, 3, releaseLater(data), 0, true);
 
         int before = connection.local().flowController().unconsumedBytes(stream);
-        channel.writeOutbound(new DefaultHttp2WindowUpdateFrame(100).setStreamId(stream.id()));
+        ChannelFuture f = channel.write(new DefaultHttp2WindowUpdateFrame(100).setStreamId(stream.id()));
         int after = connection.local().flowController().unconsumedBytes(stream);
         assertEquals(100, before - after);
+        assertTrue(f.isSuccess());
+    }
+
+    @Test
+    public void windowUpdateMayFail() throws Exception {
+        frameListener.onHeadersRead(http2HandlerCtx, 3, request, 31, false);
+        Http2Connection connection = framingCodec.connectionHandler().connection();
+        Http2Stream stream = connection.stream(3);
+        assertNotNull(stream);
+
+        // Fails, cause trying to return too many bytes to the flow controller
+        ChannelFuture f = channel.write(new DefaultHttp2WindowUpdateFrame(100).setStreamId(stream.id()));
+        assertTrue(f.isDone());
+        assertFalse(f.isSuccess());
+        assertThat(f.cause(), instanceOf(Http2Exception.class));
     }
 
     private static ChannelPromise anyChannelPromise() {