Skip to content

Commit f8545b1

Browse files
committed
Propagate failures when summary is requested
Calls `StatementResult#summary()` and `StatementResultCursor#summaryAsync()` should propagate unconsumed query errors. They are in this sense same as `#consume()` and `#consumeAsync()` respectively. Previously summary calls simply ignored existing unconsumed failure. This commit makes them propagate it instead. Note that it's later still possible to access summary after failure is consumed.
1 parent 70a2ea6 commit f8545b1

File tree

5 files changed

+103
-17
lines changed

5 files changed

+103
-17
lines changed

driver/src/main/java/org/neo4j/driver/internal/handlers/PullAllResponseHandler.java

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -142,9 +142,7 @@ public synchronized CompletionStage<Record> peekAsync()
142142
{
143143
if ( failure != null )
144144
{
145-
Throwable error = failure;
146-
failure = null; // propagate failure only once
147-
return failedFuture( error );
145+
return failedFuture( extractFailure() );
148146
}
149147

150148
if ( finished )
@@ -166,17 +164,16 @@ public synchronized CompletionStage<Record> peekAsync()
166164

167165
public synchronized CompletionStage<Record> nextAsync()
168166
{
169-
return peekAsync().thenApply( record ->
170-
{
171-
dequeueRecord();
172-
return record;
173-
} );
167+
return peekAsync().thenApply( ignore -> dequeueRecord() );
174168
}
175169

176-
// todo: propagate failure from here as well
177170
public synchronized CompletionStage<ResultSummary> summaryAsync()
178171
{
179-
if ( summary != null )
172+
if ( failure != null )
173+
{
174+
return failedFuture( extractFailure() );
175+
}
176+
else if ( summary != null )
180177
{
181178
return completedFuture( summary );
182179
}
@@ -194,9 +191,7 @@ public synchronized CompletionStage<Throwable> failureAsync()
194191
{
195192
if ( failure != null )
196193
{
197-
Throwable error = failure;
198-
failure = null; // propagate failure only once
199-
return completedFuture( error );
194+
return completedFuture( extractFailure() );
200195
}
201196
else if ( finished )
202197
{
@@ -237,6 +232,18 @@ private Record dequeueRecord()
237232
return record;
238233
}
239234

235+
private Throwable extractFailure()
236+
{
237+
if ( failure == null )
238+
{
239+
throw new IllegalStateException( "Can't consume failure because it does not exist" );
240+
}
241+
242+
Throwable error = failure;
243+
failure = null; // propagate failure only once
244+
return error;
245+
}
246+
240247
private void completeRecordFuture( Record record )
241248
{
242249
if ( recordFuture != null )

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -836,6 +836,24 @@ public void shouldBePossibleToConsumeResultAfterSessionIsClosed()
836836
assertEquals( 20000, ints.size() );
837837
}
838838

839+
@Test
840+
public void shouldPropagateFailureFromSummary()
841+
{
842+
StatementResultCursor cursor = getBlocking( session.runAsync( "RETURN Something" ) );
843+
844+
try
845+
{
846+
getBlocking( cursor.summaryAsync() );
847+
fail( "Exception expected" );
848+
}
849+
catch ( ClientException e )
850+
{
851+
assertThat( e.code(), containsString( "SyntaxError" ) );
852+
}
853+
854+
assertNotNull( getBlocking( cursor.summaryAsync() ) );
855+
}
856+
839857
private Future<List<CompletionStage<Record>>> runNestedQueries( StatementResultCursor inputCursor )
840858
{
841859
CompletableFuture<List<CompletionStage<Record>>> resultFuture = new CompletableFuture<>();

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

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1252,6 +1252,27 @@ public void shouldBePossibleToConsumeResultAfterSessionIsClosed()
12521252
assertEquals( 20000, ints.size() );
12531253
}
12541254

1255+
@Test
1256+
public void shouldPropagateFailureFromSummary()
1257+
{
1258+
try ( Session session = neo4j.driver().session() )
1259+
{
1260+
StatementResult result = session.run( "RETURN Wrong" );
1261+
1262+
try
1263+
{
1264+
result.summary();
1265+
fail( "Exception expected" );
1266+
}
1267+
catch ( ClientException e )
1268+
{
1269+
assertThat( e.code(), containsString( "SyntaxError" ) );
1270+
}
1271+
1272+
assertNotNull( result.summary() );
1273+
}
1274+
}
1275+
12551276
private void assumeServerIs31OrLater()
12561277
{
12571278
ServerVersion serverVersion = ServerVersion.version( neo4j.driver() );

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

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -323,8 +323,7 @@ public void shouldAllowRollbackAfterCoupleCorrectAndSingleWrongStatement()
323323
StatementResultCursor cursor3 = await( tx.runAsync( "RETURN" ) );
324324
try
325325
{
326-
// todo: use summaryAsync()
327-
await( cursor3.consumeAsync() );
326+
await( cursor3.summaryAsync() );
328327
fail( "Exception expected" );
329328
}
330329
catch ( Exception e )
@@ -1219,6 +1218,26 @@ public void shouldRollbackWhenPullAllFailureIsConsumed()
12191218
assertNull( getBlocking( tx.rollbackAsync() ) );
12201219
}
12211220

1221+
@Test
1222+
public void shouldPropagateFailureFromSummary()
1223+
{
1224+
Transaction tx = getBlocking( session.beginTransactionAsync() );
1225+
1226+
StatementResultCursor cursor = getBlocking( tx.runAsync( "RETURN Wrong" ) );
1227+
1228+
try
1229+
{
1230+
getBlocking( cursor.summaryAsync() );
1231+
fail( "Exception expected" );
1232+
}
1233+
catch ( ClientException e )
1234+
{
1235+
assertThat( e.code(), containsString( "SyntaxError" ) );
1236+
}
1237+
1238+
assertNotNull( getBlocking( cursor.summaryAsync() ) );
1239+
}
1240+
12221241
private int countNodes( Object id )
12231242
{
12241243
StatementResult result = session.run( "MATCH (n:Node {id: $id}) RETURN count(n)", parameters( "id", id ) );

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

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
*/
1919
package org.neo4j.driver.v1.integration;
2020

21-
import org.junit.Assert;
2221
import org.junit.Rule;
2322
import org.junit.Test;
2423
import org.junit.rules.ExpectedException;
@@ -35,9 +34,11 @@
3534
import org.neo4j.driver.v1.exceptions.ClientException;
3635
import org.neo4j.driver.v1.util.TestNeo4jSession;
3736

37+
import static org.hamcrest.CoreMatchers.containsString;
3838
import static org.hamcrest.CoreMatchers.equalTo;
3939
import static org.hamcrest.MatcherAssert.assertThat;
4040
import static org.junit.Assert.assertFalse;
41+
import static org.junit.Assert.assertNotNull;
4142
import static org.junit.Assert.assertTrue;
4243
import static org.junit.Assert.fail;
4344

@@ -357,9 +358,29 @@ public void shouldRollBackTxIfErrorWithConsume() throws Throwable
357358
StatementResult cursor = tx.run( "RETURN 1" );
358359
int val = cursor.single().get( "1" ).asInt();
359360

360-
Assert.assertThat( val, equalTo( 1 ) );
361+
assertThat( val, equalTo( 1 ) );
361362
}
362363
}
364+
}
365+
366+
@Test
367+
public void shouldPropagateFailureFromSummary()
368+
{
369+
try ( Transaction tx = session.beginTransaction() )
370+
{
371+
StatementResult result = tx.run( "RETURN Wrong" );
363372

373+
try
374+
{
375+
result.summary();
376+
fail( "Exception expected" );
377+
}
378+
catch ( ClientException e )
379+
{
380+
assertThat( e.code(), containsString( "SyntaxError" ) );
381+
}
382+
383+
assertNotNull( result.summary() );
384+
}
364385
}
365386
}

0 commit comments

Comments
 (0)