Skip to content

Conversation

kitimark
Copy link

@kitimark kitimark commented Sep 6, 2025

About the changes

Client.Ready() no longer exposes the internal one-shot uc.ready channel. It now returns a fresh channel per call that resolves by waiting on uc.onReady (a close-only broadcast). This ensures Ready() delivers even when called after the client has already become ready.

  • Affected versions: v5.0.3 and earlier
  • Unaffected APIs: WaitForReady() and listener callbacks

Closes #203

Important files

  • client.go — change to Ready() to use uc.onReady and per-call channel.
  • client_test.go — tests for initial and post-ready calls with timeouts.

Discussion points

  • Currently repository (and in streaming, the delta processor) can signal readiness into uc.ready more than once. Closing uc.onReady once makes duplicate sources harmless, but confirm this is the intended single “ready” moment across modes.
  • Returning a fresh channel per call changes channel identity. Unlikely anyone relied on the old identity, but it follows the functionality from docs.

@FredrikOseberg FredrikOseberg moved this from New to In Progress in Issues and PRs Sep 9, 2025
@FredrikOseberg FredrikOseberg moved this from In Progress to New in Issues and PRs Sep 11, 2025
@FredrikOseberg
Copy link
Contributor

@kitimark Thanks for the PR. I'm currently looking at it. So if I understand you correctly you are calling client.Ready() multiple times and the second time you call it it will hang because the channel is closed?

@youssefkhdessouky
Copy link

youssefkhdessouky commented Sep 26, 2025

@FredrikOseberg I encountered this issue during development when working with unleash-client-go. Specifically, when calling unleash.WaitForReady(), the application becomes stuck if the Unleash URL is incorrect or the Unleash server/API is unavailable. This happens because WaitForReady() blocks until the client successfully connects and retrieves the initial feature toggle state, and in cases where the connection cannot be established, it never proceeds.

@FredrikOseberg
Copy link
Contributor

@FredrikOseberg I encountered this issue during development when working with unleash-client-go. Specifically, when calling unleash.WaitForReady(), the application becomes stuck if the Unleash URL is incorrect or the Unleash server/API is unavailable. This happens because WaitForReady() blocks until the client successfully connects and retrieves the initial feature toggle state, and in cases where the connection cannot be established, it never proceeds.

This PR does not address WaitForReady though. And that would be the intended behavior of WaitForReady, in case you have feature flags that must present before the application starts. You should be able to have WaitForReady resolve by reading a backup file on disk or providing bootstrapped values. It doesn't need to depend on a network call to resolve, it just needs to have feature flags in the repository and ready to serve.

@FredrikOseberg FredrikOseberg moved this from New to Investigating in Issues and PRs Oct 9, 2025
@kitimark
Copy link
Author

@kitimark Thanks for the PR. I'm currently looking at it. So if I understand you correctly you are calling client.Ready() multiple times and the second time you call it it will hang because the channel is closed?

In our failing case we only call client.Ready() once. The problem shows up when the client finishes bootstrapping before we even start listening to the channel. The root cause is the repo signals readiness straight into uc.ready, the sync loop consumes that value immediately, so our later <-client.Ready() call just blocks. We wrap it in a timeout to guard against bad URLs or flaky networks, and that timeout is what trips, even though the client is already ready.

The reproduce code in the Issue forces that ordering so it happens every time, but it mirrors what we’ve observed in practice when the network responds quickly.

The proposed fix is workaround for makes Ready() behave like the docs suggest: each call ties into the onReady latch, so it delivers instantly if we subscribe after the client is ready, and multiple sources signalling readiness remain harmless.

Longer-term we should consider about uc.ready from this discussion point on this pull request description

Currently repository (and in streaming, the delta processor) can signal readiness into uc.ready more than once. Closing uc.onReady once makes duplicate sources harmless, but confirm this is the intended single “ready” moment across modes.

If this uc.ready abstraction is correct. we should fix this instead to make it signal readiness at once and handle it correct following the client.Ready is provided.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Investigating

Development

Successfully merging this pull request may close these issues.

Client.Ready() blocks forever when called after client is already ready

3 participants