Skip to content

Allow server to cancel when deadline expires #999

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

Merged
merged 1 commit into from
May 13, 2025

Conversation

jhump
Copy link
Member

@jhump jhump commented May 13, 2025

This makes some assertions for timeout cases a little more lenient. The tests previously asserted that a server would return a "deadline exceeded" error when the client's deadline expires. However, not all gRPC servers behave this way. The gRPC Go reference implementation was recently updated to sometimes just cancel the stream instead of sending back an error response. This doesn't seem unreasonable, so instead of updating the "known failing" list of cases for the grpc-go server, this PR makes this behavior allowed (which means it will be allowed in any other implementation-under-test, too).

The test case structure lets us define other acceptable error codes for test cases, however we don't actually want to add "canceled" as generally acceptable for these tests. It's really only acceptable for the server (clients should always report "deadline exceeded" for these cases). So we need to update the test assertions logic to get that level of nuance.

If you look at the latest commit on main as well as all open Depend-a-bot PRs, you'll see that CI is red on these commits because of this behavior -- it seems like one (out of five) of the timeout tests fails with each run. Interestingly, they normally pass (I've yet to even see a run where more than one failed; they seem to always pass when I run the suite locally). The actual failures observed so far have been in "unary", "client-stream", and "half-duplex/bidi-stream" tests (not yet observed for "server-stream" or "full-fuplex/bidi-stream" tests).

The behavior causing CI to be flaky/red was introduced in #993, which pulled in the latest grpc-go, v1.72.0, which included grpc/grpc-go#8071 (which is the culprit for the new behavior).

@jhump jhump requested a review from smaye81 May 13, 2025 13:21
Copy link

github-actions bot commented May 13, 2025

The latest Buf updates on your PR. Results from workflow Buf CI / buf (pull_request).

BuildFormatLintBreakingUpdated (UTC)
✅ passed✅ passed✅ passed✅ passedMay 13, 2025, 1:21 PM

@jhump jhump force-pushed the jh/allow-server-to-cancel-when-deadline-expires branch from 755c03c to 41515bb Compare May 13, 2025 13:21
@jhump jhump merged commit 6667276 into main May 13, 2025
5 checks passed
@jhump jhump deleted the jh/allow-server-to-cancel-when-deadline-expires branch May 13, 2025 14:26
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