Browse Source

Make ActionListener.wrap(Runnable) more Predictable (#70375)

The current implementation behaved in a non-obvious manner.
If the runnable throws but is executed in `onResponse` then it will
actually be executed twice and the exception be lost.
This means that the exception during the first call will simply be quietly swallowed.
in case the runnable (as is often the case) is a `RunOnce` or similar, where the second
execution in `onFailure` is a noop this means any exception is completely swallowed.
Armin Braun 4 years ago
parent
commit
318ae89e14
1 changed files with 27 additions and 1 deletions
  1. 27 1
      server/src/main/java/org/elasticsearch/action/ActionListener.java

+ 27 - 1
server/src/main/java/org/elasticsearch/action/ActionListener.java

@@ -232,7 +232,33 @@ public interface ActionListener<Response> {
      * @return a listener that listens for responses and invokes the runnable when received
      */
     static <Response> ActionListener<Response> wrap(Runnable runnable) {
-        return wrap(r -> runnable.run(), e -> runnable.run());
+        return new ActionListener<>() {
+            @Override
+            public void onResponse(Response response) {
+                try {
+                    runnable.run();
+                } catch (RuntimeException e) {
+                    assert false : e;
+                    throw e;
+                }
+            }
+
+            @Override
+            public void onFailure(Exception e) {
+                try {
+                    runnable.run();
+                } catch (RuntimeException ex) {
+                    ex.addSuppressed(e);
+                    assert false : ex;
+                    throw ex;
+                }
+            }
+
+            @Override
+            public String toString() {
+                return "RunnableWrappingActionListener{" + runnable + "}";
+            }
+        };
     }
 
     /**