Skip to content

Commit 9139204

Browse files
committed
Remove circular references in exceptions generated by client errors the Async Driver
The error issue happened because the cursor failure is already the cause of the commit or rollback failure. This generates a circular reference when both of the exceptions are combined. This situation can create stack overflow when the exception is logged in some logging libraries. The implemented solution is not combine two exceptions with it has any causuality between both. Stacktrace sample: ``` org.neo4j.driver.exceptions.ClientException: Transaction can't be committed. It has been rolled back either because of an error or explicit termination at org.neo4j.driver.internal.async.UnmanagedTransaction.doCommitAsync(UnmanagedTransaction.java:256) at org.neo4j.driver.internal.async.UnmanagedTransaction.lambda$commitAsync$1(UnmanagedTransaction.java:179) at java.base/java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1072) at java.base/java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:506) at java.base/java.util.concurrent.CompletableFuture.completeExceptionally(CompletableFuture.java:2088) at org.neo4j.driver.internal.handlers.pulln.AutoPullResponseHandler.failSummaryFuture(AutoPullResponseHandler.java:312) at org.neo4j.driver.internal.handlers.pulln.AutoPullResponseHandler.handleFailure(AutoPullResponseHandler.java:126) at org.neo4j.driver.internal.handlers.pulln.AutoPullResponseHandler.lambda$installRecordAndSummaryConsumers$1(AutoPullResponseHandler.java:105) at org.neo4j.driver.internal.handlers.pulln.BasicPullResponseHandler.complete(BasicPullResponseHandler.java:219) at org.neo4j.driver.internal.handlers.pulln.BasicPullResponseHandler.completeWithFailure(BasicPullResponseHandler.java:109) at org.neo4j.driver.internal.handlers.pulln.BasicPullResponseHandler$State$3.onFailure(BasicPullResponseHandler.java:345) at org.neo4j.driver.internal.handlers.pulln.BasicPullResponseHandler.onFailure(BasicPullResponseHandler.java:82) at org.neo4j.driver.internal.async.inbound.InboundMessageDispatcher.handleIgnoredMessage(InboundMessageDispatcher.java:153) at org.neo4j.driver.internal.messaging.common.CommonMessageReader.unpackIgnoredMessage(CommonMessageReader.java:88) at org.neo4j.driver.internal.messaging.common.CommonMessageReader.read(CommonMessageReader.java:62) at org.neo4j.driver.internal.async.inbound.InboundMessageHandler.channelRead0(InboundMessageHandler.java:83) at org.neo4j.driver.internal.async.inbound.InboundMessageHandler.channelRead0(InboundMessageHandler.java:35) at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:99) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324) at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) at org.neo4j.driver.internal.async.inbound.MessageDecoder.channelRead(MessageDecoder.java:47) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324) at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.channel.DefaultChannelPipeline$HeadContext.channelRead(DefaultChannelPipeline.java:1410) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.DefaultChannelPipeline.fireChannelRead(DefaultChannelPipeline.java:919) at io.netty.channel.nio.AbstractNioByteChannel$NioByteUnsafe.read(AbstractNioByteChannel.java:166) at io.netty.channel.nio.NioEventLoop.processSelectedKey(NioEventLoop.java:719) at io.netty.channel.nio.NioEventLoop.processSelectedKeysOptimized(NioEventLoop.java:655) at io.netty.channel.nio.NioEventLoop.processSelectedKeys(NioEventLoop.java:581) at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:493) at io.netty.util.concurrent.SingleThreadEventExecutor$4.run(SingleThreadEventExecutor.java:989) at io.netty.util.internal.ThreadExecutorMap$2.run(ThreadExecutorMap.java:74) at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) at java.base/java.lang.Thread.run(Thread.java:834) Caused by: org.neo4j.driver.exceptions.ClientException: The old parameter syntax `{param}` is no longer supported. Please use `$param` instead (line 1, column 34 (offset: 33)) "MATCH (n:Element) WHERE n.name = {param} RETURN n" ^ at org.neo4j.driver.internal.util.ErrorUtil.newNeo4jError(ErrorUtil.java:85) at org.neo4j.driver.internal.async.inbound.InboundMessageDispatcher.handleFailureMessage(InboundMessageDispatcher.java:108) at org.neo4j.driver.internal.messaging.common.CommonMessageReader.unpackFailureMessage(CommonMessageReader.java:83) at org.neo4j.driver.internal.messaging.common.CommonMessageReader.read(CommonMessageReader.java:59) at org.neo4j.driver.internal.async.inbound.InboundMessageHandler.channelRead0(InboundMessageHandler.java:83) at org.neo4j.driver.internal.async.inbound.InboundMessageHandler.channelRead0(InboundMessageHandler.java:35) at io.netty.channel.SimpleChannelInboundHandler.channelRead(SimpleChannelInboundHandler.java:99) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324) at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:296) at org.neo4j.driver.internal.async.inbound.MessageDecoder.channelRead(MessageDecoder.java:47) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:379) at io.netty.channel.AbstractChannelHandlerContext.invokeChannelRead(AbstractChannelHandlerContext.java:365) at io.netty.channel.AbstractChannelHandlerContext.fireChannelRead(AbstractChannelHandlerContext.java:357) at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:324) at io.netty.handler.codec.ByteToMessageDecoder.fireChannelRead(ByteToMessageDecoder.java:311) at io.netty.handler.codec.ByteToMessageDecoder.callDecode(ByteToMessageDecoder.java:432) at io.netty.handler.codec.ByteToMessageDecoder.channelRead(ByteToMessageDecoder.java:276) ... 16 more ```
1 parent a179e47 commit 9139204

File tree

5 files changed

+165
-1
lines changed

5 files changed

+165
-1
lines changed

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

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -205,7 +205,11 @@ public static CompletionException combineErrors( Throwable error1, Throwable err
205205
{
206206
Throwable cause1 = completionExceptionCause( error1 );
207207
Throwable cause2 = completionExceptionCause( error2 );
208-
addSuppressed( cause1, cause2 );
208+
if (cause1 != cause2.getCause() && cause1.getCause() != cause2) {
209+
addSuppressed( cause1, cause2 );
210+
} else if (cause1 == cause2.getCause()) {
211+
return asCompletionException( cause2 );
212+
}
209213
return asCompletionException( cause1 );
210214
}
211215
else if ( error1 != null )

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,13 @@
1818
*/
1919
package org.neo4j.driver.integration;
2020

21+
import org.junit.jupiter.api.Assertions;
2122
import org.junit.jupiter.api.Test;
2223
import org.junit.jupiter.api.extension.RegisterExtension;
24+
import org.junit.jupiter.api.function.Executable;
2325

26+
import java.io.PrintWriter;
27+
import java.io.StringWriter;
2428
import java.util.Collections;
2529
import java.util.Iterator;
2630
import java.util.List;
@@ -36,8 +40,10 @@
3640
import static java.util.Arrays.asList;
3741
import static org.hamcrest.CoreMatchers.equalTo;
3842
import static org.hamcrest.MatcherAssert.assertThat;
43+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
3944
import static org.junit.jupiter.api.Assertions.assertThrows;
4045
import static org.neo4j.driver.Values.parameters;
46+
import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
4147

4248
@ParallelizableIT
4349
class QueryIT
@@ -182,4 +188,18 @@ void shouldFailForIllegalQueries()
182188
assertThrows( IllegalArgumentException.class, () -> session.run( (String) null ) );
183189
assertThrows( IllegalArgumentException.class, () -> session.run( "" ) );
184190
}
191+
192+
@Test
193+
void shouldBeAbleToLogSemanticWrongExceptions() {
194+
try {
195+
// When I run a query with the old syntax
196+
session.writeTransaction(tx ->
197+
tx.run( "MATCH (n:Element) WHERE n.name = {param} RETURN n",
198+
parameters("param", "Luke" )).list());
199+
} catch ( Exception ex ) {
200+
// And exception happens
201+
// Then it should not have circular reference
202+
assertNoCircularReferences(ex);
203+
}
204+
}
185205
}
Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,81 @@
1+
/*
2+
* Copyright (c) "Neo4j"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.integration.async;
20+
21+
import org.junit.jupiter.api.AfterEach;
22+
import org.junit.jupiter.api.Assertions;
23+
import org.junit.jupiter.api.BeforeEach;
24+
import org.junit.jupiter.api.Test;
25+
import org.junit.jupiter.api.extension.RegisterExtension;
26+
import org.slf4j.Logger;
27+
import org.slf4j.LoggerFactory;
28+
import reactor.core.publisher.Flux;
29+
import reactor.core.publisher.Mono;
30+
31+
import java.io.PrintWriter;
32+
import java.io.StringWriter;
33+
import java.util.ArrayList;
34+
import java.util.concurrent.ExecutionException;
35+
36+
import org.neo4j.driver.async.AsyncSession;
37+
import org.neo4j.driver.util.DatabaseExtension;
38+
import org.neo4j.driver.util.ParallelizableIT;
39+
40+
import static org.neo4j.driver.Values.parameters;
41+
import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
42+
43+
@ParallelizableIT
44+
public class AsyncQueryIT
45+
{
46+
private static final Logger LOGGER = LoggerFactory.getLogger( AsyncQueryIT.class );
47+
48+
@RegisterExtension
49+
static final DatabaseExtension neo4j = new DatabaseExtension();
50+
51+
private AsyncSession session;
52+
53+
@BeforeEach
54+
void setUp()
55+
{
56+
session = neo4j.driver().asyncSession();
57+
}
58+
59+
@AfterEach
60+
void tearDown()
61+
{
62+
session.closeAsync();
63+
}
64+
65+
@Test
66+
void shouldBeAbleToLogSemanticWrongExceptions() throws ExecutionException, InterruptedException
67+
{
68+
session.writeTransactionAsync( tx -> Flux.from(
69+
Mono.fromCompletionStage(
70+
tx.runAsync( "MATCH (n:Element) WHERE n.name = {param} RETURN n", parameters("param", "Luke") )
71+
)).collectList().toFuture())
72+
73+
.toCompletableFuture()
74+
.exceptionally( ex -> {
75+
assertNoCircularReferences(ex);
76+
return new ArrayList<>();
77+
} )
78+
.get();
79+
}
80+
81+
}

driver/src/test/java/org/neo4j/driver/internal/util/FuturesTest.java

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import org.junit.jupiter.api.Test;
2828

2929
import java.io.IOException;
30+
import java.util.Optional;
3031
import java.util.concurrent.CancellationException;
3132
import java.util.concurrent.CompletableFuture;
3233
import java.util.concurrent.CompletionException;
@@ -39,13 +40,16 @@
3940
import static org.hamcrest.Matchers.is;
4041
import static org.hamcrest.junit.MatcherAssert.assertThat;
4142
import static org.junit.jupiter.api.Assertions.assertArrayEquals;
43+
import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
4244
import static org.junit.jupiter.api.Assertions.assertEquals;
4345
import static org.junit.jupiter.api.Assertions.assertFalse;
46+
import static org.junit.jupiter.api.Assertions.assertNotEquals;
4447
import static org.junit.jupiter.api.Assertions.assertNull;
4548
import static org.junit.jupiter.api.Assertions.assertThrows;
4649
import static org.junit.jupiter.api.Assertions.assertTrue;
4750
import static org.neo4j.driver.internal.util.Matchers.blockingOperationInEventLoopError;
4851
import static org.neo4j.driver.util.DaemonThreadFactory.daemon;
52+
import static org.neo4j.driver.util.TestUtil.assertNoCircularReferences;
4953
import static org.neo4j.driver.util.TestUtil.sleep;
5054

5155
class FuturesTest
@@ -310,6 +314,37 @@ void shouldCombineTwoErrors()
310314
assertArrayEquals( new Throwable[]{error2Cause}, combined.getCause().getSuppressed() );
311315
}
312316

317+
@Test
318+
void shouldCombineTwoErrorsUsingErrorCause2()
319+
{
320+
RuntimeException error1 = new RuntimeException( "Error1" );
321+
RuntimeException error2Cause = new RuntimeException( "Error2", error1 );
322+
CompletionException error2 = new CompletionException( error2Cause );
323+
324+
CompletionException combined = Futures.combineErrors( error1, error2 );
325+
326+
assertEquals( error2Cause, combined.getCause() );
327+
328+
assertNoCircularReferences( combined );
329+
assertArrayEquals( new Throwable[]{}, combined.getCause().getSuppressed() );
330+
}
331+
332+
@Test
333+
void shouldCombineTwoErrorsUsingError1()
334+
{
335+
RuntimeException error2Cause = new RuntimeException( "Error2" );
336+
RuntimeException error1 = new RuntimeException( "Error1", error2Cause );
337+
CompletionException error2 = new CompletionException( error2Cause );
338+
339+
CompletionException combined = Futures.combineErrors( error1, error2 );
340+
341+
assertEquals( error1, combined.getCause() );
342+
343+
assertNoCircularReferences( combined );
344+
assertArrayEquals( new Throwable[]{}, combined.getCause().getSuppressed() );
345+
}
346+
347+
313348
@Test
314349
void shouldCombineErrorAndNull()
315350
{

driver/src/test/java/org/neo4j/driver/util/TestUtil.java

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import reactor.core.publisher.Mono;
3030

3131
import java.time.Duration;
32+
import java.util.ArrayList;
3233
import java.util.Arrays;
3334
import java.util.HashSet;
3435
import java.util.LinkedHashSet;
@@ -644,6 +645,29 @@ public static ServerVersion anyServerVersion()
644645
return ServerVersion.v4_0_0;
645646
}
646647

648+
public static void assertNoCircularReferences(Throwable ex)
649+
{
650+
assertNoCircularReferences( ex, new ArrayList<>() );
651+
}
652+
653+
private static void assertNoCircularReferences(Throwable ex, List<Throwable> list)
654+
{
655+
list.add( ex );
656+
if (ex.getCause() != null ) {
657+
if (list.contains( ex.getCause() )) {
658+
throw new AssertionError("Circular reference detected", ex.getCause());
659+
}
660+
assertNoCircularReferences(ex.getCause(), list);
661+
}
662+
for ( Throwable suppressed: ex.getSuppressed() )
663+
{
664+
if(list.contains( suppressed )) {
665+
throw new AssertionError("Circular reference detected", suppressed);
666+
}
667+
assertNoCircularReferences( suppressed, list );
668+
}
669+
}
670+
647671
private static void setupSuccessfulPullAll( Connection connection, String query )
648672
{
649673
doAnswer( invocation ->

0 commit comments

Comments
 (0)