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

onReady() listener doesn't work when fresh installing the app having pooling disabled. #88

Open
jnavarro98 opened this issue Sep 12, 2024 · 6 comments · Fixed by #90
Open
Assignees
Labels
documentation Improvements or additions to documentation

Comments

@jnavarro98
Copy link

jnavarro98 commented Sep 12, 2024

Describe the bug

Initializing unleash with .pollingStrategy.enabled(false) when fresh installing the app doesn't trigger onReady() listener.

However if the app was installed and had pooling enabled, then changed to pooling disabled, onReady() is called correctly.
It seems that in order to have the listeners working, you have to have pooling enabled at least once.

This makes .pollingStrategy.enabled(false) useless for our use case (we block the app launch relying in this listener).

Our workaround was to have pooling enabled with .pollingStrategy.interval(Long.MAX_VALUE)

Steps to reproduce the bug

Have unleash initialized with .pollingStrategy.enabled(false)

Expected behavior

onReady() is called when sdk is initialized

Unleash version

1.0.1

Subscription type

Enterprise

Hosting type

Self-hosted

@jnavarro98 jnavarro98 added the bug Something isn't working label Sep 12, 2024
@jnavarro98 jnavarro98 changed the title onReady() listener doesn't work when fresh installing the app and having pooling disabled. onReady() listener doesn't work when fresh installing the app having pooling disabled. Sep 12, 2024
@gastonfournier
Copy link
Contributor

@jnavarro98 I'm trying to understand this. If you're never polling the SDK will never be ready unless you provide a bootstrap, in which case the SDK will be ready as soon as the bootstrap file is read.

If you want to have polling disabled you need to explicitly tell unleash to do at least one fetch of the features configuration. If the SDK has not seen any configuration (either from polling or from a bootstrap) it won't be ready.

You can see many of our tests cover this use case, this is just an example:

.pollingStrategy.enabled(false)
.metricsStrategy.enabled(false)
.localStorageConfig.enabled(false)
.forceImpressionData(true)
.build(),
unleashContext = UnleashContext(userId = "123"),
lifecycle = mock(Lifecycle::class.java)
)
var ready = false
val impressionEvents = mutableListOf<ImpressionEvent>()
unleash.start(
eventListeners = listOf(object : UnleashReadyListener, UnleashImpressionEventListener {
override fun onImpression(event: ImpressionEvent) {
println("Impression event received: ${event.featureName}")
impressionEvents.add(event)
}
override fun onReady() {
ready = true
}
}), bootstrap = listOf(
Toggle(name = "with-impression-1", enabled = true, impressionData = true),
Toggle(name = "with-impression-2", enabled = true, impressionData = true),
Toggle(name = "with-impression-3", enabled = true, impressionData = true),
Toggle(name = "without-impression", enabled = false)
)
)
await().atMost(1, TimeUnit.SECONDS).until { ready }

To manually trigger 1 fetch an never poll again you have the following methods:

/**
* This function forces a refresh of the toggles from the server and wait until the refresh is complete or failed.
* Usually, this is done automatically in the background, but you can call this function to force a refresh.
*/
fun refreshTogglesNow()
/**
* This function forces a refresh of the toggles from the server asynchronously using the IO dispatcher.
* Usually, this is done automatically in the background, but you can call this function to force a refresh.
*/
fun refreshTogglesNowAsync()

Let me know if I misunderstood your request, but initially it doesn't seem like a bug.

@gastonfournier
Copy link
Contributor

We've added #90 that is a very specific test for this particular use case

@github-project-automation github-project-automation bot moved this from Support rotation to Done in Issues and PRs Sep 20, 2024
@jnavarro98
Copy link
Author

Thanks for the clarification. I found that the documentation in your website is quite lacking. When I call unleash.start I expect it to at least fetch flags, I shouldn't have to do reverse engineering to understand the behaviour of an sdk. There were many times where I had to figure out many things by looking at your code. For example the Heartbeat listener, needing to have the unleash.start in the main thread (or else it crashes) and many other features your SDK provide aren't documented.

This is not acceptable for an SDK. Please work on the documentation.

@gastonfournier
Copy link
Contributor

When I call unleash.start I expect it to at least fetch flags

Our assumption is that when you set the polling strategy to disabled, you don't want to poll (therefore you don't fetch flags), but maybe you understand it differently. I could see another way of understanding "polling disabled" is "continuous polling is disabled, but at least the SDK will fetch once". On the other hand, we have users that don't want that initial fetch because they want to control when that happens so we need to account for both.

When I call unleash.start I expect it to at least fetch flags, I shouldn't have to do reverse engineering to understand the behaviour of an sdk. There were many times where I had to figure out many things by looking at your code. For example the Heartbeat listener, needing to have the unleash.start in the main thread (or else it crashes) and many other features your SDK provide aren't documented.

This SDK is quite new, and as that the documentation is also quite fresh. We tried to support many different use cases (fetch, don't fetch, fetch once, fetch when the customer wants, etc), so that creates quite a lot of different combinations to cover. The ones that are documented in the website are the basics, but we will continue to improve them based on feedback or external contributions. If you have a snippet of what failed in your setup or how you ended up configuring your SDK for your use case we can add that, or if you have a suggestion how to make the polling configuration clearer we can also look into that.

@jnavarro98
Copy link
Author

At least have documented every function we can use from unleash.

For our use case, we wanted to block the application start until unleash is ready and do a single fetch at start, we used a suspend coroutine. That is why it was important to know that if we disabled the pooling it wouldn't be called unless we pass the bootstrap and we would be blocked waiting for a callback that never comes.

Now that we disabled pooling and added the bootstrap, we also had to fetch the toggles manually (which seems redundant and adds complexity, start could know when to do the fetch, in my opinion if you want to start unleash you want to get the flag values).

It would be useful to add the case of pooling disabled in the documentation, what do you need to do. Many developers don't want to alter the status of their feature flags in runtime since it adds complexity and gives almost no value in our case.

Screenshot 2024-09-27 at 11 52 59 Screenshot 2024-09-27 at 11 48 09

@gastonfournier
Copy link
Contributor

Thanks for the snippets! I'll try to do that next week and share a PR here.

At least have documented every function we can use from unleash.

Software is an iterative process and we'll get there eventually, but it's a moving target. Every piece of help we get from the community such as what you did, helps us make the product better. Thank you!

@gastonfournier gastonfournier added documentation Improvements or additions to documentation and removed bug Something isn't working labels Sep 27, 2024
@github-project-automation github-project-automation bot moved this from Done to New in Issues and PRs Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
Status: For later
Development

Successfully merging a pull request may close this issue.

2 participants