Non-stop cohort sync polling

[gtramontina]

As I followed the instructions in the docs, and I had debug enabled, I noticed that it was logging [Experiment] cohort polled, changed: false non-stop.

A bit of digging led me to:

https://github.com/amplitude/experiment-node-server/blob/01b9e7f86e228fd38609c4f77968b212e9f0a4ba/packages/node/src/local/client.ts#L128C9-L131C11

Then I chatted with co-pilot to confirm my understanding and it replied:

If this.config.cohortSyncConfig exists but does not define cohortPollingIntervalMillis, then:

this.config.cohortSyncConfig?.cohortPollingIntervalMillis evaluates to undefined (because the property is missing).
So the expression becomes effectively:

Math.max(COHORT_POLLING_INTERVAL_MILLIS_MIN, undefined)

In JavaScript/TypeScript, Math.max(...) converts its arguments to numbers. undefined converts to NaN, and if any argument is NaN, the result of Math.max is NaN.

So the result of that whole expression will be NaN, not the minimum constant.

This was effectively polling without any interval.

Furthermore, I don't think this would act as a "default". If, for example, I had set my config to have cohortPollingIntervalMillis = 1_000 to poll every second, the snippet above would pick the max of both, which would be the constant 60_000.

[tyiuhc]

@gtramontina, many thanks for raising this issue. A fix has been released v1.13.4 - if cohortPollingIntervalMillis is not defined, then the polling interval will use the default of 60s.

Furthermore, I don't think this would act as a "default". If, for example, I had set my config to have cohortPollingIntervalMillis = 1_000 to poll every second, the snippet above would pick the max of both, which would be the constant 60_000.

This is the intended behavior, used to set the polling interval to a minimum of 60s.