Improve stacktraces for errors from blocking API

Blocking API works on top of async API and simply blocks on the
returned futures. Exceptions can thus contain stacktraces unrelated
to the user's code because they originate from driver worker threads.
Such exceptions make it hard for users of the driver to determine what
has caused the error.

This commit makes exceptions thrown from blocking API calls contain
stacktraces of the current thread. Original async stacktrace is
attached in a dummy suppressed exception.

Author: lutovich

driver/src/main/java/org/neo4j/driver/internal/util/ErrorUtil.java

@@ -18,6 +18,11 @@
  */
 package org.neo4j.driver.internal.util;
 
+import io.netty.util.internal.PlatformDependent;
+
+import java.util.concurrent.ExecutionException;
+import java.util.stream.Stream;
+
 import org.neo4j.driver.v1.exceptions.AuthenticationException;
 import org.neo4j.driver.v1.exceptions.ClientException;
 import org.neo4j.driver.v1.exceptions.DatabaseException;
@@ -85,6 +90,21 @@ public static boolean isFatal( Throwable error )
         return true;
     }
 
+    public static void rethrowAsyncException( ExecutionException e )
+    {
+        Throwable error = e.getCause();
+
+        InternalExceptionCause internalCause = new InternalExceptionCause( error.getStackTrace() );
+        error.addSuppressed( internalCause );
+
+        StackTraceElement[] currentStackTrace = Stream.of( Thread.currentThread().getStackTrace() )
+                .skip( 2 ) // do not include Thread.currentThread() and this method in the stacktrace
+                .toArray( StackTraceElement[]::new );
+        error.setStackTrace( currentStackTrace );
+
+        PlatformDependent.throwException( error );
+    }
+
     private static boolean isProtocolViolationError( Neo4jException error )
     {
         String errorCode = error.code();
@@ -106,4 +126,24 @@ private static String extractClassification( String code )
         }
         return parts[1];
     }
+
+    /**
+     * Exception which is merely a holder of an async stacktrace, which is not the primary stacktrace users are interested in.
+     * Used for blocking API calls that block on async API calls.
+     */
+    private static class InternalExceptionCause extends RuntimeException
+    {
+        InternalExceptionCause( StackTraceElement[] stackTrace )
+        {
+            setStackTrace( stackTrace );
+        }
+
+        @Override
+        public synchronized Throwable fillInStackTrace()
+        {
+            // no need to fill in the stack trace
+            // this exception just uses the given stack trace
+            return this;
+        }
+    }
 }

driver/src/main/java/org/neo4j/driver/internal/util/Futures.java

@@ -18,8 +18,6 @@
  */
 package org.neo4j.driver.internal.util;
 
-import io.netty.util.internal.PlatformDependent;
-
 import java.util.concurrent.CompletableFuture;
 import java.util.concurrent.CompletionException;
 import java.util.concurrent.CompletionStage;
@@ -121,7 +119,7 @@ public static <V> V blockingGet( CompletionStage<V> stage, Runnable interruptHan
                 }
                 catch ( ExecutionException e )
                 {
-                    PlatformDependent.throwException( e.getCause() );
+                    ErrorUtil.rethrowAsyncException( e );
                 }
             }
         }

driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java

@@ -20,13 +20,18 @@
 
 import io.netty.channel.Channel;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInfo;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import java.io.IOException;
+import java.lang.reflect.Method;
 import java.net.URI;
+import java.util.Arrays;
 import java.util.List;
+import java.util.Objects;
 import java.util.UUID;
 import java.util.function.Consumer;
+import java.util.stream.Stream;
 
 import org.neo4j.driver.internal.cluster.RoutingSettings;
 import org.neo4j.driver.internal.messaging.response.FailureMessage;
@@ -46,10 +51,13 @@
 import org.neo4j.driver.v1.exceptions.ServiceUnavailableException;
 import org.neo4j.driver.v1.util.SessionExtension;
 
+import static java.util.Arrays.asList;
 import static java.util.concurrent.TimeUnit.SECONDS;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.startsWith;
 import static org.hamcrest.Matchers.containsString;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
+import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -228,6 +236,20 @@ void shouldCloseChannelOnInboundFatalFailureMessage() throws InterruptedExceptio
         assertEquals( queryError.getMessage(), errorMessage );
     }
 
+    @Test
+    void shouldThrowErrorWithNiceStackTrace( TestInfo testInfo )
+    {
+        ClientException error = assertThrows( ClientException.class, () -> session.run( "RETURN 10 / 0" ).consume() );
+
+        // thrown error should have this class & method in the stacktrace
+        StackTraceElement[] stackTrace = error.getStackTrace();
+        assertTrue( Stream.of( stackTrace ).anyMatch( element -> testClassAndMethodMatch( testInfo, element ) ),
+                () -> "Expected stacktrace element is absent:\n" + Arrays.toString( stackTrace ) );
+
+        // thrown error should have a suppressed error with an async stacktrace
+        assertThat( asList( error.getSuppressed() ), hasSize( greaterThanOrEqualTo( 1 ) ) );
+    }
+
     private Throwable testChannelErrorHandling( Consumer<FailingMessageFormat> messageFormatSetup )
             throws InterruptedException
     {
@@ -276,4 +298,23 @@ private void assertNewQueryCanBeExecuted( Session session, ChannelTrackingDriver
         Channel lastChannel = channels.get( channels.size() - 1 );
         assertTrue( lastChannel.isActive() );
     }
+
+    private static boolean testClassAndMethodMatch( TestInfo testInfo, StackTraceElement element )
+    {
+        return testClassMatches( testInfo, element ) && testMethodMatches( testInfo, element );
+    }
+
+    private static boolean testClassMatches( TestInfo testInfo, StackTraceElement element )
+    {
+        String expectedName = testInfo.getTestClass().map( Class::getName ).orElse( "" );
+        String actualName = element.getClassName();
+        return Objects.equals( expectedName, actualName );
+    }
+
+    private static boolean testMethodMatches( TestInfo testInfo, StackTraceElement element )
+    {
+        String expectedName = testInfo.getTestMethod().map( Method::getName ).orElse( "" );
+        String actualName = element.getMethodName();
+        return Objects.equals( expectedName, actualName );
+    }
 }

driver/src/test/java/org/neo4j/driver/v1/integration/TransactionIT.java

@@ -22,7 +22,6 @@
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
-import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
@@ -45,9 +44,11 @@
 import org.neo4j.driver.v1.util.StubServer;
 import org.neo4j.driver.v1.util.TestUtil;
 
+import static java.util.Arrays.asList;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.Matchers.greaterThanOrEqualTo;
 import static org.hamcrest.Matchers.instanceOf;
 import static org.hamcrest.junit.MatcherAssert.assertThat;
 import static org.junit.jupiter.api.Assertions.assertEquals;
@@ -368,7 +369,7 @@ void shouldDisallowQueriesAfterFailureWhenResultsAreConsumed()
         try ( Transaction tx = session.beginTransaction() )
         {
             List<Integer> xs = tx.run( "UNWIND [1,2,3] AS x CREATE (:Node) RETURN x" ).list( record -> record.get( 0 ).asInt() );
-            assertEquals( Arrays.asList( 1, 2, 3 ), xs );
+            assertEquals( asList( 1, 2, 3 ), xs );
 
             ClientException error1 = assertThrows( ClientException.class, () -> tx.run( "RETURN unknown" ).consume() );
             assertThat( error1.code(), containsString( "SyntaxError" ) );
@@ -404,7 +405,7 @@ void shouldRollbackWhenMarkedSuccessfulButOneStatementFails()
         } );
 
         assertThat( error.code(), containsString( "SyntaxError" ) );
-        assertEquals( 1, error.getSuppressed().length );
+        assertThat( error.getSuppressed().length, greaterThanOrEqualTo( 1 ) );
         Throwable suppressed = error.getSuppressed()[0];
         assertThat( suppressed, instanceOf( ClientException.class ) );
         assertThat( suppressed.getMessage(), startsWith( "Transaction can't be committed" ) );