SSE data source can wipe in-memory feature map: UpdateFromApiResponse overwrites features with nil (unlike loadData) #70

Summary

When using the SSE data source (WithSseDataSource()), a features SSE event whose payload has a missing/null features field will wipe the in-memory feature map of a running client. After that, EvalFeature returns Source: unknownFeature for every flag (and Features() returns empty) until the process restarts — even though EnsureLoaded() previously succeeded.

The root cause is an asymmetry: the initial/reconnect load path guards against a nil feature set, but the live SSE-event path does not.

Affected versions

Confirmed in v0.2.9 (latest) and reproduced back through v0.2.4. The relevant code is unchanged across these releases.

Root cause

(*Client).UpdateFromApiResponse assigns d.features = features unconditionally:

// client.go
func (client *Client) UpdateFromApiResponse(resp *FeatureApiResponse) error {
    ...
    } else {
        features = resp.Features   // nil when the response omits "features"
    }
    client.data.withLock(func(d *data) error {
        d.features = features       // <-- wipes the map when features == nil
        d.savedGroups = resp.SavedGroups
        d.dateUpdated = resp.DateUpdated
        return nil
    })
    return nil
}

The SSE live-event path reaches this with no nil check:

// datasource_sse.go
func (ds *SseDataSource) processEvent(event sse.Event) {
    if event.Data == "" {
        return
    }
    ds.logger.Info("Updating features")
    err := ds.client.UpdateFromApiResponseJSON(event.Data) // -> UpdateFromApiResponse, no guard
    ...
}

By contrast, the initial/reconnect load path does guard:

// datasource_sse.go  (loadData)
if resp.Features == nil {
    return nil
}
err = ds.client.UpdateFromApiResponse(resp)

So an empty/malformed features event (or any event payload that unmarshals to Features == nil) silently clears a previously-populated map, whereas the same payload over the load path is correctly ignored.

Impact

For consumers that use feature flags as safety switches, this is a silent fail mode: EvalFeature(...).On flips to false (Source: unknownFeature) for all flags on the affected client, indistinguishable from a flag being genuinely off, with no error returned. It persists for the life of the process. We hit this in production behind a single-replica service, where it took down 100% of that pod's flag evaluations until restart.

Reproduction (sketch)

1. Start a client with WithSseDataSource(); EnsureLoaded() succeeds and Features() is populated.
2. Have the SSE endpoint emit a features event whose JSON has no features key (or "features": null) but a dateUpdated >= the current one.
3. processEvent -> UpdateFromApiResponseJSON -> UpdateFromApiResponse sets d.features = nil.
4. Every subsequent EvalFeature returns Source: unknownFeature, On: false; Features() is empty.

(Equivalently, calling UpdateFromApiResponseJSON("{\"dateUpdated\":\"<now>\"}") on a loaded client wipes the map.)

Proposed fix

Apply the same guard that loadData already uses, at the authoritative point in UpdateFromApiResponse, so all callers (SSE events, polling, manual) are protected:

} else {
    features = resp.Features
}
if features == nil {
    // Never overwrite a populated map with an empty payload.
    client.logger.Warn("Api response contains no features, refuse to update")
    return nil
}

This also makes the existing nil-check in loadData redundant (harmless), and keeps the empty-map state reachable only via the explicit SetFeatures(FeatureMap{}) API.

Happy to send a PR if the maintainers agree with guarding inside UpdateFromApiResponse vs. only in processEvent.