Presently, if polling for configuration hits an error (such as a timeout), polling will stop. However, some polling errors should be able to be tolerated to handle things like network or CDN hiccups.
Tip for Reviewers
Review this PR with the "Hide Whitespace" option:
Description
This PR adds two main things:
Retries for the initial request of the configuration. These are separated by a short wait (~10% of the polling interval) as initialization is typically awaited so we don't want it to take too long.
Retries for polling for updated configurations. These are separated by an exponential backoff, and if a certain amount fails, polling will stop.
It also adds the ability to have the initialization not throw an error on failure, and for it to attempt polling after a failed initial request. (E.g., initialize the server with no configurations, but leave the chance to get one later)
This is all controlled by newly added configuration options:
numInitialRequestRetries - how many times fetching the initial configuration in init() will be retried (default: 1; current is effectively 0)
throwOnFailedInitialization - whether or not init() should throw/reject with an error if no initial configuration was successfully requested (default: true; which is current behavior)
pollAfterFailedInitialization - whether or not to start regular polling for configurations if one is not loaded in init(). (default: false; which is current behavior)
numPollRequestRetries - how many times a failed request for an updated configuration (after init() is done) will be attempted. (default: 7*; current is effectively 0)
*Note that 7 retries means the last retry will be about 254 intervals after the failure. With our current interval of 30 seconds, this will be about two hours.
I also exposed requestTimeoutMs as an init() configuration option as it was already configurable downstream.
If we like these changes, I think we should move the poller to the common JS library and have these options for our client SDK as well, but perhaps with different defaults (e.g., throwOnFailedInitialization default to false).
🎟️ Ticket: FF-1426 - Node SDK - Non Fatal Polling
Motivation and Context
Presently, if polling for configuration hits an error (such as a timeout), polling will stop. However, some polling errors should be able to be tolerated to handle things like network or CDN hiccups.
Tip for Reviewers
Review this PR with the "Hide Whitespace" option:
Description
This PR adds two main things:
await
ed so we don't want it to take too long.It also adds the ability to have the initialization not throw an error on failure, and for it to attempt polling after a failed initial request. (E.g., initialize the server with no configurations, but leave the chance to get one later)
This is all controlled by newly added configuration options:
numInitialRequestRetries
- how many times fetching the initial configuration ininit()
will be retried (default: 1; current is effectively 0)throwOnFailedInitialization
- whether or notinit()
should throw/reject with an error if no initial configuration was successfully requested (default: true; which is current behavior)pollAfterFailedInitialization
- whether or not to start regular polling for configurations if one is not loaded ininit()
. (default: false; which is current behavior)numPollRequestRetries
- how many times a failed request for an updated configuration (afterinit()
is done) will be attempted. (default: 7*; current is effectively 0)*Note that 7 retries means the last retry will be about 254 intervals after the failure. With our current interval of 30 seconds, this will be about two hours.
I also exposed
requestTimeoutMs
as aninit()
configuration option as it was already configurable downstream.If we like these changes, I think we should move the poller to the common JS library and have these options for our client SDK as well, but perhaps with different defaults (e.g.,
throwOnFailedInitialization
default tofalse
).How has this been tested?
Expansion of
poller.spec
andindex.spec