Issue #4855 - Occasional h2spec failures on CI

Fixed notification of HTTP2Session.abort(), that must fail all the streams before failing the session. Signed-off-by: Simone Bordet <[email protected]>
jetty · Jun 5, 2020 · 82529e6 · 82529e6
1 parent 8d60636
commit 82529e6
Show file tree

Hide file tree

Showing 8 changed files with 60 additions and 22 deletions.
diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Session.java
@@ -498,14 +498,15 @@ public void onWindowUpdate(WindowUpdateFrame frame)
     public void onStreamFailure(int streamId, int error, String reason)
     {
         Callback callback = new ResetCallback(streamId, error, Callback.NOOP);
-        onStreamFailure(streamId, error, reason, callback);
+        Throwable failure = toFailure("Stream failure", error, reason);
+        onStreamFailure(streamId, error, reason, failure, callback);
     }
 
-    private void onStreamFailure(int streamId, int error, String reason, Callback callback)
+    private void onStreamFailure(int streamId, int error, String reason, Throwable failure, Callback callback)
     {
         IStream stream = getStream(streamId);
         if (stream != null)
-            stream.process(new FailureFrame(error, reason), callback);
+            stream.process(new FailureFrame(error, reason, failure), callback);
         else
             callback.succeeded();
     }
@@ -517,31 +518,48 @@ public void onConnectionFailure(int error, String reason)
     }
 
     protected void onConnectionFailure(int error, String reason, Callback callback)
+    {
+        Throwable failure = toFailure("Session failure", error, reason);
+        onFailure(error, reason, failure, new CloseCallback(error, reason, callback));
+    }
+
+    protected void abort(Throwable failure)
+    {
+        onFailure(ErrorCode.NO_ERROR.code, null, failure, new TerminateCallback(failure));
+    }
+
+    private void onFailure(int error, String reason, Throwable failure, Callback callback)
     {
         Collection<Stream> streams = getStreams();
         int count = streams.size();
-        Callback countCallback = new CountingCallback(new CloseCallback(error, reason, callback), count + 1);
+        Callback countCallback = new CountingCallback(callback, count + 1);
         for (Stream stream : streams)
         {
-            onStreamFailure(stream.getId(), error, reason, countCallback);
+            onStreamFailure(stream.getId(), error, reason, failure, countCallback);
         }
-        IOException failure = new IOException(String.format("%s/%s", ErrorCode.toString(error, null), reason));
         notifyFailure(this, failure, countCallback);
     }
 
     private void onClose(GoAwayFrame frame, Callback callback)
     {
+        int error = frame.getError();
+        String reason = frame.tryConvertPayload();
+        Throwable failure = toFailure("Session close", error, reason);
         Collection<Stream> streams = getStreams();
         int count = streams.size();
         Callback countCallback = new CountingCallback(callback, count + 1);
-        String reason = frame.tryConvertPayload();
         for (Stream stream : streams)
         {
-            onStreamFailure(stream.getId(), frame.getError(), reason, countCallback);
+            onStreamFailure(stream.getId(), error, reason, failure, countCallback);
         }
         notifyClose(this, frame, countCallback);
     }
 
+    private Throwable toFailure(String message, int error, String reason)
+    {
+        return new IOException(String.format("%s %s/%s", message, ErrorCode.toString(error, null), reason));
+    }
+
     @Override
     public void newStream(HeadersFrame frame, Promise<Stream> promise, Stream.Listener listener)
     {
@@ -1024,11 +1042,6 @@ private void terminate(Throwable cause)
         }
     }
 
-    protected void abort(Throwable failure)
-    {
-        notifyFailure(this, failure, new TerminateCallback(failure));
-    }
-
     public boolean isDisconnected()
     {
         return !endPoint.isOpen();

diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/HTTP2Stream.java
@@ -609,7 +609,7 @@ private void notifyFailure(Stream stream, FailureFrame frame, Callback callback)
         {
             try
             {
-                listener.onFailure(stream, frame.getError(), frame.getReason(), callback);
+                listener.onFailure(stream, frame.getError(), frame.getReason(), frame.getFailure(), callback);
             }
             catch (Throwable x)
             {

diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/api/Stream.java
@@ -227,8 +227,24 @@ default boolean onIdleTimeout(Stream stream, Throwable x)
          * @param stream the stream
          * @param error the error code
          * @param reason the error reason, or null
+         * @param failure the failure
          * @param callback the callback to complete when the failure has been handled
          */
+        default void onFailure(Stream stream, int error, String reason, Throwable failure, Callback callback)
+        {
+            onFailure(stream, error, reason, callback);
+        }
+
+        /**
+         * <p>Callback method invoked when the stream failed.</p>
+         *
+         * @param stream the stream
+         * @param error the error code
+         * @param reason the error reason, or null
+         * @param callback the callback to complete when the failure has been handled
+         * @deprecated use {@link #onFailure(Stream, int, String, Throwable, Callback)} instead
+         */
+        @Deprecated
         default void onFailure(Stream stream, int error, String reason, Callback callback)
         {
             callback.succeeded();

diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FailureFrame.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/frames/FailureFrame.java
@@ -22,12 +22,14 @@ public class FailureFrame extends Frame
 {
     private final int error;
     private final String reason;
+    private final Throwable failure;
 
-    public FailureFrame(int error, String reason)
+    public FailureFrame(int error, String reason, Throwable failure)
     {
         super(FrameType.FAILURE);
         this.error = error;
         this.reason = reason;
+        this.failure = failure;
     }
 
     public int getError()
@@ -39,4 +41,9 @@ public String getReason()
     {
         return reason;
     }
+
+    public Throwable getFailure()
+    {
+        return failure;
+    }
 }
diff --git a/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/Generator.java b/jetty-http2/http2-common/src/main/java/org/eclipse/jetty/http2/generator/Generator.java
@@ -56,7 +56,7 @@ public Generator(ByteBufferPool byteBufferPool, int maxDynamicTableSize, int max
         this.generators[FrameType.WINDOW_UPDATE.getType()] = new WindowUpdateGenerator(headerGenerator);
         this.generators[FrameType.CONTINUATION.getType()] = null; // Never generated explicitly.
         this.generators[FrameType.PREFACE.getType()] = new PrefaceGenerator();
-        this.generators[FrameType.DISCONNECT.getType()] = new DisconnectGenerator();
+        this.generators[FrameType.DISCONNECT.getType()] = new NoOpGenerator();
 
         this.dataGenerator = new DataGenerator(headerGenerator);
     }

diff --git a/.../http2/generator/DisconnectGenerator.java → .../jetty/http2/generator/NoOpGenerator.java b/.../http2/generator/DisconnectGenerator.java → .../jetty/http2/generator/NoOpGenerator.java
@@ -21,9 +21,9 @@
 import org.eclipse.jetty.http2.frames.Frame;
 import org.eclipse.jetty.io.ByteBufferPool;
 
-public class DisconnectGenerator extends FrameGenerator
+public class NoOpGenerator extends FrameGenerator
 {
-    public DisconnectGenerator()
+    public NoOpGenerator()
     {
         super(null);
     }

diff --git a/...nt-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java b/...nt-transport/src/main/java/org/eclipse/jetty/http2/client/http/HttpReceiverOverHTTP2.java
@@ -195,9 +195,9 @@ public boolean onIdleTimeout(Stream stream, Throwable x)
     }
 
     @Override
-    public void onFailure(Stream stream, int error, String reason, Callback callback)
+    public void onFailure(Stream stream, int error, String reason, Throwable failure, Callback callback)
     {
-        responseFailure(new IOException(String.format("%s/%s", ErrorCode.toString(error, null), reason)));
+        responseFailure(failure);
         callback.succeeded();
     }
 

diff --git a/...tp2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java b/...tp2-server/src/main/java/org/eclipse/jetty/http2/server/HTTP2ServerConnectionFactory.java
@@ -34,6 +34,7 @@
 import org.eclipse.jetty.http2.frames.ResetFrame;
 import org.eclipse.jetty.io.EndPoint;
 import org.eclipse.jetty.io.EofException;
+import org.eclipse.jetty.io.QuietException;
 import org.eclipse.jetty.server.Connector;
 import org.eclipse.jetty.server.HttpConfiguration;
 import org.eclipse.jetty.server.NegotiatingServerConnection.CipherDiscriminator;
@@ -160,9 +161,10 @@ public void onReset(Stream stream, ResetFrame frame, Callback callback)
         }
 
         @Override
-        public void onFailure(Stream stream, int error, String reason, Callback callback)
+        public void onFailure(Stream stream, int error, String reason, Throwable failure, Callback callback)
         {
-            EofException failure = new EofException(String.format("Failure %s/%s", ErrorCode.toString(error, null), reason));
+            if (!(failure instanceof QuietException))
+                failure = new EofException(failure);
             onFailure(stream, failure, callback);
         }
-Original file line number
+Diff line change
@@ Expand Up @@
             {
                 try
                 {
-                    listener.onFailure(stream, frame.getError(), frame.getReason(), callback);
+                    listener.onFailure(stream, frame.getError(), frame.getReason(), frame.getFailure(), callback);
                 }
                 catch (Throwable x)
                 {
@@ Expand Down @@