Skip to content

Conversation

@davidhcoe
Copy link
Contributor

@davidhcoe davidhcoe commented Aug 28, 2025

The previous update was still not catching the issue. Found some older grpc classes with the similar message (No current element is available.) and pattern including https://github.com/grpc/grpc/blob/2d4f3c56001cd1e1f85734b2f7c5ce5f2797c38a/src/csharp/Grpc.Core/Internal/ServerRequestStream.cs#L38, so added a try/catch around the attempt to add an additional check. Included some mocks to test as well.

@github-actions github-actions bot added this to the ADBC Libraries 20 milestone Aug 28, 2025
@davidhcoe davidhcoe marked this pull request as draft August 28, 2025 13:20
@davidhcoe davidhcoe marked this pull request as ready for review August 29, 2025 03:34
Copy link
Contributor

@CurtHagenlocher CurtHagenlocher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! I'm going to check this in; consider adding logging for the swallowed exception as a followup.

}
}
else
catch (InvalidOperationException)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we're swallowing this exception, should we at least write something to the logs (e.g. stack trace) to ensure it's not entirely lost?

int maxStreamCount,
Activity? activity)
{
ReadSession rs = new ReadSession { Table = table, DataFormat = DataFormat.Arrow };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suspect that moving the session creation into the retriable part is probably what will fix this problem.

@CurtHagenlocher CurtHagenlocher merged commit 3452994 into apache:main Sep 3, 2025
6 checks passed
@davidhcoe davidhcoe deleted the dev/readchunk-fix-redeux branch October 24, 2025 10:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants