Weird exception handling when using `RunAsync` directly on a session

[Aaronmsv]

Hi,

I've come across some weird exception handling when using RunAsync directly on a session. I was writing an integration test that expected a ClientException because of unique constraint and that's where I encountered the issue. There's no exception coming from RunAsync, but the test does crash later on. I would guess that the query is executed on another thread, which causes the exception to throw on another thread instead of the calling thread, or something similar.

Since it was just a test, I would use RunAsync directly on a session instead of calling ExecuteWriteAsync and then using the IAsyncQueryRunner. I didn't need e.g. automatic retry in the test.

Environment information

Neo4j Version: 5.1.0 Community Neo4j Mode: Single instance Driver version: .NET driver 5.1.0 and 5.2.0 Operating System: Linux 6.0.9-arch1-1 x86_64

Steps to reproduce

I've included an xUnit test below that demonstrates the problem. I explicitly use try/catch here, instead of Assert.ThrowsAsync, to reduce the number of potentially contributing factors.

[Fact]
public async Task BugReport()
{
    await using IAsyncSession session = CreateSession();
    await session.RunAsync(
        "CREATE CONSTRAINT example_constraint IF NOT EXISTS FOR (d:SomeLabel) REQUIRE d.SomeProperty IS UNIQUE");
    const string query = @"
        CREATE (:SomeLabel {SomeProperty: $value})
        CREATE (:SomeLabel {SomeProperty: $value})";
    string value = Guid.NewGuid().ToString("N");
    var parameters = new {value};

    try
    {
        await session.RunAsync(query, parameters);
    }
    catch (ClientException clientEx)
    {
        Assert.Equal("Neo.ClientError.Schema.ConstraintValidationFailed", clientEx.Code);
    }
    catch (Exception)
    {
        Assert.Fail("Expected ClientException");
    }
}

I have omitted the code for CreateSession(), as it is an implementation detail on how it resolves the config and driver instance, but it's basically something like this:

private IAsyncSession CreateSession()
{
    return driver.AsyncSession();
}

Expected behavior

The first catch block gets executed.

Actual behavior

The test crashes:

Neo4j.Driver.ClientException
Node(0) already exists with label `SomeLabel` and property `SomeProperty` = '0a67a8cf1cad4b82b1a73510a32e4708'
   at Neo4j.Driver.Internal.MessageHandling.ResponsePipelineError.EnsureThrownIf(Func`2 predicate)
   at Neo4j.Driver.Internal.MessageHandling.ResponsePipelineError.EnsureThrown()
   at Neo4j.Driver.Internal.Result.ResultCursorBuilder.ConsumeAsync()
   at Neo4j.Driver.Internal.AsyncSession.DiscardUnconsumedResultAsync()
   at Neo4j.Driver.Internal.AsyncSession.DisposeSessionResultAsync()
   at Neo4j.Driver.Internal.AsyncSession.DisposeSessionResultAsync()
   at Neo4j.Driver.Internal.AsyncSession.<CloseAsync>b__50_0()
   at Neo4j.Driver.Internal.Logging.DriverLoggerUtil.TryExecuteAsync(ILogger logger, Func`1 func, String message)
   at Neo4j.Driver.Internal.AsyncSession.DisposeAsyncCore()
   at Neo4j.Driver.Internal.AsyncQueryRunner.DisposeAsync()
   at $Points to the closing brace of the last catch block in the test method$
   at Xunit.Sdk.TestInvoker`1.<>c__DisplayClass48_0.<<InvokeTestMethodAsync>b__1>d.MoveNext() in /_/src/xunit.execution/Sdk/Frameworks/Runners/TestInvoker.cs:line 264
--- End of stack trace from previous location ---
   at Xunit.Sdk.ExecutionTimer.AggregateAsync(Func`1 asyncAction) in /_/src/xunit.execution/Sdk/Frameworks/ExecutionTimer.cs:line 48
   at Xunit.Sdk.ExceptionAggregator.RunAsync(Func`1 code) in /_/src/xunit.core/Sdk/ExceptionAggregator.cs:line 90

Workaround

In the test, replace the following line:

await session.RunAsync(query, parameters);

With:

await session.ExecuteWriteAsync(async queryRunner => await queryRunner.RunAsync(query, parameters));

[AndyHeap-NeoTech]

Hi Aaronmsv.
session.RunAsync returns a IResultCursor which represents the results stream. The error will only surface/throw when you fetch from that stream. In your test the correct thing to do is consume the result cursor.

var resultCursor = await session.RunAsync(query, parameters); resultCursor.Consume();

I suspect crash you are seeing currently is occuring when the result cursor returned by RunAsync goes out of scope and is disposed, perhaps in conflict of the session also being disposed at the same time.

When using ExecuteWriteAsync, the disposal of the result cursor happens when the lambda finishes thus taking place at the correct time and raising the exception you expect.

[Aaronmsv]

Ah I see. So in general, when using session.RunAsync, you are responsible for the result cursor; and when you use ExecuteWriteAsync, then it handles the result cursor for you (although both methods return IResultCursor)?

Is it best to always do resultCursor.Consume();?

[AndyHeap-NeoTech]

I wouldn't describe it as ExecuteWriteAsync is handling the cursor for you, it is just scoped differently as it is with in the callback lambda that you supply. We consider it best practice to call Consume when finished using a result cursor, even in the ExecuteRead/ExecuteWrite methods.

[AndyHeap-NeoTech]

I'll close the issue. Please feel free to re-open if needed.