Remove circular references in exceptions during the commit

driver/src/main/java/org/neo4j/driver/internal/async/UnmanagedTransaction.java

@@ -121,24 +121,29 @@ boolean isOpen()
 
     private volatile StateHolder state = StateHolder.of( State.ACTIVE );
 
-    public UnmanagedTransaction(Connection connection, BookmarkHolder bookmarkHolder, long fetchSize )
+    public UnmanagedTransaction( Connection connection, BookmarkHolder bookmarkHolder, long fetchSize )
+    {
+        this( connection, bookmarkHolder, fetchSize, new ResultCursorsHolder() );
+    }
+
+    protected UnmanagedTransaction( Connection connection, BookmarkHolder bookmarkHolder, long fetchSize, ResultCursorsHolder resultCursors )
     {
         this.connection = connection;
         this.protocol = connection.protocol();
         this.bookmarkHolder = bookmarkHolder;
-        this.resultCursors = new ResultCursorsHolder();
+        this.resultCursors = resultCursors;
         this.fetchSize = fetchSize;
     }
 
-    public CompletionStage<UnmanagedTransaction> beginAsync(Bookmark initialBookmark, TransactionConfig config )
+    public CompletionStage<UnmanagedTransaction> beginAsync( Bookmark initialBookmark, TransactionConfig config )
     {
         return protocol.beginTransaction( connection, initialBookmark, config )
-                .handle( ( ignore, beginError ) ->
-                {
-                    if ( beginError != null )
-                    {
-                        if ( beginError instanceof AuthorizationExpiredException )
-                        {
+                       .handle( ( ignore, beginError ) ->
+                                {
+                                    if ( beginError != null )
+                                    {
+                                        if ( beginError instanceof AuthorizationExpiredException )
+                                        {
                             connection.terminateAndRelease( AuthorizationExpiredException.DESCRIPTION );
                         }
                         else
@@ -176,7 +181,7 @@ else if ( state.value == State.ROLLED_BACK )
         else
         {
             return resultCursors.retrieveNotConsumedError()
-                                .thenCompose( error -> doCommitAsync().handle( handleCommitOrRollback( error ) ) )
+                                .thenCompose( error -> doCommitAsync( error ).handle( handleCommitOrRollback( error ) ) )
                                 .whenComplete( ( ignore, error ) -> handleTransactionCompletion( State.COMMITTED, error ) );
         }
     }
@@ -249,12 +254,13 @@ else if ( state.value == State.TERMINATED )
         }
     }
 
-    private CompletionStage<Void> doCommitAsync()
+    private CompletionStage<Void> doCommitAsync( Throwable cursorFailure )
     {
         if ( state.value == State.TERMINATED )
         {
             return failedFuture( new ClientException( "Transaction can't be committed. " +
-                                                      "It has been rolled back either because of an error or explicit termination", state.causeOfTermination ) );
+                                                      "It has been rolled back either because of an error or explicit termination",
+                                                      cursorFailure != state.causeOfTermination ? state.causeOfTermination : null ) );
         }
         return protocol.commitTransaction( connection ).thenAccept( bookmarkHolder::setBookmark );
     }

driver/src/test/java/org/neo4j/driver/integration/BookmarkIT.java

@@ -18,20 +18,17 @@
  */
 package org.neo4j.driver.integration;
 
-import org.hamcrest.BaseMatcher;
-import org.hamcrest.Description;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
 
 import java.util.UUID;
 
+import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.Session;
 import org.neo4j.driver.Transaction;
 import org.neo4j.driver.exceptions.ClientException;
-import org.neo4j.driver.Bookmark;
-import org.neo4j.driver.internal.InternalBookmark;
 import org.neo4j.driver.internal.util.DisabledOnNeo4jWith;
 import org.neo4j.driver.internal.util.EnabledOnNeo4jWith;
 import org.neo4j.driver.internal.util.Neo4jFeature;
@@ -47,6 +44,7 @@
 import static org.neo4j.driver.internal.util.BookmarkUtil.assertBookmarkContainsSingleValue;
 import static org.neo4j.driver.internal.util.BookmarkUtil.assertBookmarkIsEmpty;
 import static org.neo4j.driver.internal.util.BookmarkUtil.assertBookmarksContainsSingleUniqueValues;
+import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
 
 @ParallelizableIT
 class BookmarkIT
@@ -137,7 +135,8 @@ void bookmarkRemainsAfterTxFailure()
         Transaction tx = session.beginTransaction();
         tx.run( "RETURN" );
 
-        assertThrows( ClientException.class, tx::commit );
+        ClientException e = assertThrows( ClientException.class, tx::commit );
+        assertNoCircularReferences( e );
         assertEquals( bookmark, session.lastBookmark() );
     }
 

driver/src/test/java/org/neo4j/driver/integration/QueryIT.java

@@ -18,9 +18,13 @@
  */
 package org.neo4j.driver.integration;
 
+import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 import org.junit.jupiter.api.extension.RegisterExtension;
+import org.junit.jupiter.api.function.Executable;
 
+import java.io.PrintWriter;
+import java.io.StringWriter;
 import java.util.Collections;
 import java.util.Iterator;
 import java.util.List;
@@ -36,8 +40,10 @@
 import static java.util.Arrays.asList;
 import static org.hamcrest.CoreMatchers.equalTo;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.neo4j.driver.Values.parameters;
+import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
 
 @ParallelizableIT
 class QueryIT
@@ -182,4 +188,18 @@ void shouldFailForIllegalQueries()
         assertThrows( IllegalArgumentException.class, () -> session.run( (String) null ) );
         assertThrows( IllegalArgumentException.class, () -> session.run( "" ) );
     }
+
+    @Test
+    void shouldBeAbleToLogSemanticWrongExceptions() {
+        try {
+            // When I run a query with the old syntax
+            session.writeTransaction(tx ->
+                                             tx.run( "MATCH (n:Element) WHERE n.name = {param} RETURN n",
+                                                     parameters("param", "Luke" )).list());
+        } catch ( Exception ex ) {
+            // And exception happens
+            // Then it should not have circular reference
+            assertNoCircularReferences(ex);
+        }
+    }
 }

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

@@ -29,8 +29,8 @@
 import org.neo4j.driver.Config;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.Record;
-import org.neo4j.driver.Session;
 import org.neo4j.driver.Result;
+import org.neo4j.driver.Session;
 import org.neo4j.driver.Transaction;
 import org.neo4j.driver.Value;
 import org.neo4j.driver.exceptions.ClientException;
@@ -57,6 +57,7 @@
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.neo4j.driver.internal.logging.DevNullLogging.DEV_NULL_LOGGING;
 import static org.neo4j.driver.internal.retry.RetrySettings.DEFAULT;
+import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
 
 @ParallelizableIT
 class TransactionIT
@@ -291,7 +292,8 @@ void shouldRollBackTxIfErrorWithoutConsume()
         Transaction tx = session.beginTransaction();
         tx.run( "invalid" ); // send run, pull_all
 
-        assertThrows( ClientException.class, tx::commit );
+        ClientException e = assertThrows( ClientException.class, tx::commit );
+        assertNoCircularReferences( e );
 
         try ( Transaction anotherTx = session.beginTransaction() )
         {
@@ -385,7 +387,8 @@ void shouldBeResponsiveToThreadInterruptWhenWaitingForCommit()
 
             try
             {
-                assertThrows( ServiceUnavailableException.class, tx2::commit );
+                ServiceUnavailableException e = assertThrows( ServiceUnavailableException.class, tx2::commit );
+                assertNoCircularReferences( e );
             }
             finally
             {
@@ -481,6 +484,7 @@ void shouldRollbackWhenMarkedSuccessfulButOneQueryFails()
             }
         } );
 
+        assertNoCircularReferences( error );
         assertThat( error.code(), containsString( "SyntaxError" ) );
         assertThat( error.getSuppressed().length, greaterThanOrEqualTo( 1 ) );
         Throwable suppressed = error.getSuppressed()[0];

driver/src/test/java/org/neo4j/driver/integration/async/AsyncQueryIT.java

@@ -0,0 +1,81 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.neo4j.driver.integration.async;
+
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.RegisterExtension;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import reactor.core.publisher.Flux;
+import reactor.core.publisher.Mono;
+
+import java.io.PrintWriter;
+import java.io.StringWriter;
+import java.util.ArrayList;
+import java.util.concurrent.ExecutionException;
+
+import org.neo4j.driver.async.AsyncSession;
+import org.neo4j.driver.util.DatabaseExtension;
+import org.neo4j.driver.util.ParallelizableIT;
+
+import static org.neo4j.driver.Values.parameters;
+import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
+
+@ParallelizableIT
+public class AsyncQueryIT
+{
+    private static final Logger LOGGER = LoggerFactory.getLogger( AsyncQueryIT.class );
+
+    @RegisterExtension
+    static final DatabaseExtension neo4j = new DatabaseExtension();
+
+    private AsyncSession session;
+
+    @BeforeEach
+    void setUp()
+    {
+        session = neo4j.driver().asyncSession();
+    }
+
+    @AfterEach
+    void tearDown()
+    {
+        session.closeAsync();
+    }
+
+    @Test
+    void shouldBeAbleToLogSemanticWrongExceptions() throws ExecutionException, InterruptedException
+    {
+        session.writeTransactionAsync( tx -> Flux.from(
+                Mono.fromCompletionStage(
+                        tx.runAsync( "MATCH (n:Element) WHERE n.name = {param} RETURN n", parameters("param", "Luke") )
+                )).collectList().toFuture())
+
+               .toCompletableFuture()
+               .exceptionally( ex -> {
+                   assertNoCircularReferences(ex);
+                   return new ArrayList<>();
+               } )
+               .get();
+    }
+
+}

driver/src/test/java/org/neo4j/driver/integration/async/AsyncSessionIT.java

@@ -83,6 +83,7 @@
 import static org.neo4j.driver.internal.util.Matchers.syntaxError;
 import static org.neo4j.driver.internal.util.Neo4jFeature.BOLT_V3;
 import static org.neo4j.driver.internal.util.Neo4jFeature.BOLT_V4;
+import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
 import static org.neo4j.driver.util.TestUtil.await;
 import static org.neo4j.driver.util.TestUtil.awaitAll;
 
@@ -381,7 +382,8 @@ void shouldRunAsyncTransactionThatCanNotBeRetried()
         InvocationTrackingWork work = new InvocationTrackingWork( "UNWIND [10, 5, 0] AS x CREATE (:Hi) RETURN 10/x" );
         CompletionStage<Record> txStage = session.writeTransactionAsync( work );
 
-        assertThrows( ClientException.class, () -> await( txStage ) );
+        ClientException e = assertThrows( ClientException.class, () -> await( txStage ) );
+        assertNoCircularReferences( e );
         assertEquals( 1, work.invocationCount() );
         assertEquals( 0, countNodesByLabel( "Hi" ) );
     }

driver/src/test/java/org/neo4j/driver/integration/async/AsyncTransactionIT.java

@@ -69,6 +69,7 @@
 import static org.neo4j.driver.internal.util.Iterables.single;
 import static org.neo4j.driver.internal.util.Matchers.containsResultAvailableAfterAndResultConsumedAfter;
 import static org.neo4j.driver.internal.util.Matchers.syntaxError;
+import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
 import static org.neo4j.driver.util.TestUtil.await;
 
 @ParallelizableIT
@@ -677,6 +678,7 @@ void shouldFailToCommitWhenQueriesFailAndErrorNotConsumed() throws InterruptedEx
         tx.runAsync( "CREATE (:TestNode)" );
 
         ClientException e = assertThrows( ClientException.class, () -> await( tx.commitAsync() ) );
+        assertNoCircularReferences( e );
         assertEquals( "/ by zero", e.getMessage() );
     }
 
@@ -688,6 +690,7 @@ void shouldPropagateRunFailureFromCommit()
         tx.runAsync( "RETURN ILLEGAL" );
 
         ClientException e = assertThrows( ClientException.class, () -> await( tx.commitAsync() ) );
+        assertNoCircularReferences( e );
         assertThat( e.getMessage(), containsString( "ILLEGAL" ) );
     }
 
@@ -699,6 +702,7 @@ void shouldPropagateBlockedRunFailureFromCommit()
         await( tx.runAsync( "RETURN 42 / 0" ) );
 
         ClientException e = assertThrows( ClientException.class, () -> await( tx.commitAsync() ) );
+        assertNoCircularReferences( e );
         assertThat( e.getMessage(), containsString( "/ by zero" ) );
     }
 
@@ -732,6 +736,7 @@ void shouldPropagatePullAllFailureFromCommit()
         tx.runAsync( "UNWIND [1, 2, 3, 'Hi'] AS x RETURN 10 / x" );
 
         ClientException e = assertThrows( ClientException.class, () -> await( tx.commitAsync() ) );
+        assertNoCircularReferences( e );
         assertThat( e.code(), containsString( "TypeError" ) );
     }
 
@@ -743,6 +748,7 @@ void shouldPropagateBlockedPullAllFailureFromCommit()
         await( tx.runAsync( "UNWIND [1, 2, 3, 'Hi'] AS x RETURN 10 / x" ) );
 
         ClientException e = assertThrows( ClientException.class, () -> await( tx.commitAsync() ) );
+        assertNoCircularReferences( e );
         assertThat( e.code(), containsString( "TypeError" ) );
     }