diff --git a/driver/src/main/java/org/neo4j/driver/internal/util/ErrorUtil.java b/driver/src/main/java/org/neo4j/driver/internal/util/ErrorUtil.java index f1c40324c3..8e2ae55a5a 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/util/ErrorUtil.java +++ b/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; + } + } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/util/Futures.java b/driver/src/main/java/org/neo4j/driver/internal/util/Futures.java index a806ae59d7..649be74bd8 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/util/Futures.java +++ b/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 blockingGet( CompletionStage stage, Runnable interruptHan } catch ( ExecutionException e ) { - PlatformDependent.throwException( e.getCause() ); + ErrorUtil.rethrowAsyncException( e ); } } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java index 18427b9b68..30aef2a2ed 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/ErrorIT.java +++ b/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 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 ); + } } diff --git a/driver/src/test/java/org/neo4j/driver/v1/integration/TransactionIT.java b/driver/src/test/java/org/neo4j/driver/v1/integration/TransactionIT.java index 5153bd8b8a..3945eec68b 100644 --- a/driver/src/test/java/org/neo4j/driver/v1/integration/TransactionIT.java +++ b/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 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" ) );