Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update to v1.0.3 of conformance suite #318

Merged
merged 1 commit into from
Aug 16, 2024
Merged

Conversation

jhump
Copy link
Member

@jhump jhump commented Aug 15, 2024

Some new test cases in the latest suite revealed errors in how cancelation of unary RPCs was implemented in the conformance client, but it only impacted the "suspend" style of invocation (callback and blocking styles worked fine).

The issue is that the coroutineScope function doesn't return until all child coroutines complete. So even though the helper was using launch to create a concurrent coroutine, the whole thing blocked until that job finished. In order to fix that, I had to move the coroutineScope call higher up the call stack (out of a helper method on the UnaryClient adapter and into the conformance Client itself), so it also made sense to move the related InvokeStyle enum.

Most of the diff is inlining that helper into the conformance client. But I also had to add an exception handler to the async job, to actually complete the future (with an exception) if the coroutine gets canceled (which is the only way we expose canceling a suspend unary RPC).

…e client for 'suspend' style unary RPCs

Signed-off-by: Josh Humphries <[email protected]>
@jhump jhump force-pushed the jh/update-latest-conformance branch from 399d064 to 0d62942 Compare August 15, 2024 16:54
@jhump jhump requested a review from pkwarren August 15, 2024 17:13
@jhump jhump merged commit 09d80a2 into main Aug 16, 2024
10 checks passed
@jhump jhump deleted the jh/update-latest-conformance branch August 16, 2024 16:12
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