-
Notifications
You must be signed in to change notification settings - Fork 89
chore: Support offline mode #561
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
base: feat/fdv2
Are you sure you want to change the base?
Conversation
|
This is going to fail until I merge a pending feature and cut a release for the go sdk. |
| case <-s.quit: | ||
| return nil, context.Canceled | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Single-use data channel shared across synchronizer calls
The dataCh channel is read from by both the Fetch() and Sync() methods, but only a single data item is ever sent to this channel (in AddEnvironment). Additionally, the same OfflineModeSynchronizer instance is used for both primary and fallback synchronizers via Synchronizers(syncFactory, syncFactory). If the SDK calls Sync() on both (or calls both Fetch() and Sync()), multiple goroutines will race to receive from dataCh. Only one will succeed; the others will block forever waiting for data that never arrives, potentially causing initialization hangs.
Additional Locations (2)
| } | ||
| result.ChangeSet = &changeSet | ||
| result.State = interfaces.DataSourceStateValid | ||
| resultChan <- result |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Error state persists incorrectly across result updates
The result struct is reused across loop iterations in Sync(). When a changeSet is received (lines 150-156), the code sets result.ChangeSet and result.State but does not clear result.Error. If a previous status change set an error, that stale error will be included when sending subsequent valid changeSet updates. Consumers may incorrectly interpret the result as containing an error when the data is actually valid.
|
|
||
| p.updateHandler.AddEnvironment(testFileDataEnv2) | ||
| client2 := p.awaitClient() | ||
| assert.Equal(t, testFileDataEnv2.Params.SDKKey, client2.Key) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: Test awaits wrong environment ID after adding second env
After adding testFileDataEnv2, the test calls awaitEnvironment(testFileDataEnv1.Params.EnvID) instead of awaitEnvironment(testFileDataEnv2.Params.EnvID). This means the test doesn't actually verify that the second environment was properly initialized - it just re-checks the first environment. The test will pass even if adding the second environment fails to complete.
Note
Introduce an offline-mode DataSynchronizer that loads archive data without network calls, integrate it into relay environment setup/updates, and bump go-server-sdk to v7.14.3.
internal/filedata/offline_mode_synchronizer.goimplementingsubsystems.DataSynchronizerfor offline mode (fetches from archive via channel; broadcastsChangeSet/status; supportsUpdateData).relay/filedata_actions.go, replace stub data source with customDataSystemusingOfflineModeSynchronizer; manage per-environment synchronizers; send initialSDKData; apply updates viaUpdateData; clean up on delete.relay/filedata_actions_test.goto align with new offline environment creation/update flow (adjust sequencing in delete test).github.com/launchdarkly/go-server-sdk/v7tov7.14.3.relay/relay.go(error var grouping, signature wrap).Written by Cursor Bugbot for commit 2d754cc. This will update automatically on new commits. Configure here.