Cleanup code to make stacktrace less confusing (#796)

Motivation:

5dade1f introduced a change that did
prevent the Channel to be closed if a STREAM_STOPPED frame was received
but did not change the name of the method from forceClose(...) to
something else. This makes the stacktrace quite confusing as while the
method is called forceClose(...) it actually might not close the
channel.

Modifications:

- Move all the logic into the writable(...) method and so make things
less confusing

Result:

Less confusing stacktrace for failed writes

Author: normanmaurer

codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicChannel.java

@@ -1087,10 +1087,7 @@ private boolean handleWritableStreams(QuicheQuicConnection conn) {
                             QuicheQuicStreamChannel streamChannel = streams.get(streamId);
                             if (streamChannel != null) {
                                 int capacity = Quiche.quiche_conn_stream_capacity(connAddr, streamId);
-                                if (capacity < 0) {
-                                    // Let's close the channel if quiche_conn_stream_capacity(...) returns an error.
-                                    streamChannel.forceClose(capacity);
-                                } else if (streamChannel.writable(capacity)) {
+                                if (streamChannel.writable(capacity)) {
                                     mayNeedWrite = true;
                                 }
                             }

codec-classes-quic/src/main/java/io/netty/incubator/codec/quic/QuicheQuicStreamChannel.java

@@ -410,10 +410,31 @@ public String toString() {
     }
 
     /**
-     * Stream is writable.
+     * Stream writability changed.
      */
     boolean writable(int capacity) {
         assert eventLoop().inEventLoop();
+        if (capacity < 0) {
+            // If the value is negative its a quiche error.
+            if (capacity != Quiche.QUICHE_ERR_DONE) {
+                if (!queue.isEmpty()) {
+                    if (capacity == Quiche.QUICHE_ERR_STREAM_STOPPED) {
+                        queue.removeAndFailAll(new ChannelOutputShutdownException("STOP_SENDING frame received"));
+                        // If STOP_SENDING is received we should not close the channel but just fail all queued writes.
+                        return false;
+                    } else {
+                        queue.removeAndFailAll(Quiche.convertToException(capacity));
+                    }
+                } else if (capacity == Quiche.QUICHE_ERR_STREAM_STOPPED) {
+                    // If STOP_SENDING is received we should not close the channel
+                    return false;
+                }
+                // IF this error was not QUICHE_ERR_STREAM_STOPPED we should close the channel.
+                finSent = true;
+                unsafe().close(unsafe().voidPromise());
+            }
+            return false;
+        }
         this.capacity = capacity;
         boolean mayNeedWrite = unsafe().writeQueued();
         // we need to re-read this.capacity as writeQueued() may update the capacity.
@@ -428,25 +449,6 @@ private void updateWritabilityIfNeeded(boolean newWritable) {
         }
     }
 
-    void forceClose(int error) {
-        if (error == Quiche.QUICHE_ERR_DONE) {
-            return;
-        }
-        if (!queue.isEmpty()) {
-            if (error == Quiche.QUICHE_ERR_STREAM_STOPPED) {
-                queue.removeAndFailAll(new ChannelOutputShutdownException("STOP_SENDING frame received"));
-                // If STOP_SENDING is received we should not close the channel but just fail all queued writes.
-                return;
-            } else {
-                queue.removeAndFailAll(Quiche.convertToException(error));
-            }
-        } else if (error == Quiche.QUICHE_ERR_STREAM_STOPPED) {
-            // If STOP_SENDING is received we should not close the channel
-            return;
-        }
-        forceClose();
-    }
-
     /**
      * Stream is readable.
      */
@@ -459,13 +461,6 @@ void readable() {
         }
     }
 
-    void forceClose() {
-        assert eventLoop().inEventLoop();
-        // Set received to true to ensure we will remove it from the internal map once we send the fin.
-        finSent = true;
-        unsafe().close(unsafe().voidPromise());
-    }
-
     final class QuicStreamChannelUnsafe implements Unsafe {
 
         @SuppressWarnings("deprecation")