fix(health): bound each sidecar health request by the health timeout - #1255
Closed
nelson-parente wants to merge 1 commit into
Closed
nelson-parente wants to merge 1 commit into
nelson-parente wants to merge 1 commit into
Conversation
DaprHealth.wait_for_sidecar() called urlopen without a timeout. If the health endpoint accepted the connection but never answered, the call blocked forever, and DAPR_HEALTH_TIMEOUT was never checked. Every client constructor calls it, so client creation could hang. The async version used the aiohttp default total timeout of 300 seconds for each request. Give each request the time that is left before the deadline, with a floor of 1 second, in both versions. Signed-off-by: Nelson Parente <nelson_parente@live.com.pt> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017JhKUDV9dbs5JDz2uwZqz5
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
DaprHealth.wait_for_sidecar()polls/v1.0/healthz/outbounduntil it gets a 2xx response orDAPR_HEALTH_TIMEOUT(60 seconds by default) runs out. The deadline is checked only between requests. Each request had no limit of its own:dapr/clients/health.pycalledurllib.request.urlopenwithouttimeout. If the endpoint accepts the connection but never answers, that call blocks forever.dapr/aio/clients/health.pycalledsession.getwith the aiohttp default, a total timeout of 300 seconds for each request.The gRPC, HTTP and async clients and the streaming subscriptions call
wait_for_sidecar()when they start, so client creation can hang. CI shows this: in Windows run 36901464540 onmain, the unit tests stopped for 5 minutes intest_bulk_save_then_get_states. The main thread was inwait_for_sidecar→urlopen→_read_status, waiting on a fake sidecar socket that never answered, and the job then hit its 20-minute limit. #1242 fixes why that socket was left open in the tests. This PR fixes the SDK side: the health check now ends at its deadline in all cases.Each request now gets the time that is left before the deadline, with a floor of 1 second, so
DAPR_HEALTH_TIMEOUT=0still makes one attempt. A request that times out is handled like any other failed attempt, and the loop raisesTimeoutErrorat the deadline, as before.Issue reference
No issue.
Checklist
Tests
Two new tests, one for each version, use a listening socket on an ephemeral port that never accepts. The TCP connect succeeds, but no response comes back. With
DAPR_HEALTH_TIMEOUT=1,wait_for_sidecar()must raiseTimeoutErrorwithin 10 seconds. Before this change, the sync test was still blocked after 10 seconds, and the async test ran into the 10-second guard. With it, both raise after about 1 second.uv run pytest -m "not e2e" ./tests --ignore=tests/integration --ignore=tests/examples: 2074 passed (Python 3.14, macOS).uv run ruff check,uv run ruff format --checkanduv run mypy: clean.🤖 Generated with Claude Code
https://claude.ai/code/session_017JhKUDV9dbs5JDz2uwZqz5