Add suppressed exception to original cause when calling Future.sync* (#14898)

chrisvest · normanmaurer · web-flow · commit a7e206d82afe · 2025-04-02T08:28:16.000+02:00
Motivation:
The `Promise.sync` and `syncUninterruptibly` methods will rethrow any
exception that cause the promise to fail. This gives the stack trace of
the original failure, but people loose the stack trace telling which
specific sync call propagated the exception. This can make debugging
confusing and more difficult that necessary. We have to keep this
behavior by default for compatibility, but we can add the extra details as suppressed exception and so help with debugging.

Modification:
Add`CompletionException` when `sync` or `syncUninterruptibly` is called as suppressed exception to the original cause.

Result:
Easier to debug.

---------

Co-authored-by: Norman Maurer &lt;norman_maurer@apple.com&gt;
diff --git a/common/src/main/java/io/netty/util/concurrent/DefaultPromise.java b/common/src/main/java/io/netty/util/concurrent/DefaultPromise.java
@@ -24,6 +24,7 @@
 import io.netty.util.internal.logging.InternalLoggerFactory;
 
 import java.util.concurrent.CancellationException;
+import java.util.concurrent.CompletionException;
 import java.util.concurrent.ExecutionException;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.TimeoutException;
@@ -33,11 +34,22 @@
 import static java.util.concurrent.TimeUnit.MILLISECONDS;
 
 public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
+    /**
+     * System property with integer type value, that determine the max reentrancy/recursion level for when
+     * listener notifications prompt other listeners to be notified.
+     * <p>
+     * When the reentrancy/recursion level becomes greater than this number, a new task will instead be scheduled
+     * on the event loop, to finish notifying any subsequent listners.
+     * <p>
+     * The default value is {@code 8}.
+     */
+    public static final String PROPERTY_MAX_LISTENER_STACK_DEPTH = "io.netty.defaultPromise.maxListenerStackDepth";
+
     private static final InternalLogger logger = InternalLoggerFactory.getInstance(DefaultPromise.class);
     private static final InternalLogger rejectedExecutionLogger =
             InternalLoggerFactory.getInstance(DefaultPromise.class.getName() + ".rejectedExecution");
     private static final int MAX_LISTENER_STACK_DEPTH = Math.min(8,
-            SystemPropertyUtil.getInt("io.netty.defaultPromise.maxListenerStackDepth", 8));
+            SystemPropertyUtil.getInt(PROPERTY_MAX_LISTENER_STACK_DEPTH, 8));
     @SuppressWarnings("rawtypes")
     private static final AtomicReferenceFieldUpdater<DefaultPromise, Object> RESULT_UPDATER =
             AtomicReferenceFieldUpdater.newUpdater(DefaultPromise.class, Object.class, "result");
@@ -49,10 +61,11 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
 
     private volatile Object result;
     private final EventExecutor executor;
+
     /**
      * One or more listeners. Can be a {@link GenericFutureListener} or a {@link DefaultFutureListeners}.
      * If {@code null}, it means either 1) no listeners were added yet or 2) all listeners were notified.
-     *
+     * <p>
      * Threading - synchronized(this). We must support adding listeners when there is no EventExecutor.
      */
     private GenericFutureListener<? extends Future<?>> listener;
@@ -70,7 +83,7 @@ public class DefaultPromise<V> extends AbstractFuture<V> implements Promise<V> {
 
     /**
      * Creates a new instance.
-     *
+     * <p>
      * It is preferable to use {@link EventExecutor#newPromise()} to create a new promise
      *
      * @param executor
@@ -668,6 +681,9 @@ private void rethrowIfFailed() {
             return;
         }
 
+        if (!(cause instanceof CancellationException) && cause.getSuppressed().length == 0) {
+            cause.addSuppressed(new CompletionException("Rethrowing promise failure cause", null));
+        }
         PlatformDependent.throwException(cause);
     }
 
diff --git a/common/src/main/java/io/netty/util/concurrent/Future.java b/common/src/main/java/io/netty/util/concurrent/Future.java
@@ -153,7 +153,7 @@ public interface Future<V> extends java.util.concurrent.Future<V> {
 
     /**
      * Return the result without blocking. If the future is not done yet this will return {@code null}.
-     *
+     * <p>
      * As it is possible that a {@code null} value is used to mark the future as successful you also need to check
      * if the future is really done with {@link #isDone()} and not rely on the returned {@code null} value.
      */
