feat(sync): inject isOnlineOverride into SyncService.isOnline()#68
Open
abhijitnairDhwani wants to merge 1 commit into
Open
feat(sync): inject isOnlineOverride into SyncService.isOnline()#68abhijitnairDhwani wants to merge 1 commit into
abhijitnairDhwani wants to merge 1 commit into
Conversation
`SyncService.isOnline()` directly probes `connectivity_plus` and accepts only `mobile | wifi | ethernet`. On Android emulators with `adb reverse` the link reads as `ConnectivityResult.none` and the gate refuses sync even though HTTP traffic to the bench is fully functional. Today the only escape is to bypass `pullSync`/`pullSyncMany` entirely from the consumer app — costing delta pulls, resumable cursors, and the in-built page lookahead that `_pullOneInternal` already implements. Add an optional `isOnlineOverride` callback wired through `FrappeSDK(...)` → `SyncService(...)` → `isOnline()`. When set, the override replaces the `connectivity_plus` probe. When unset (the default), behaviour is exactly as today — fully backwards-compatible. Semantics: - The `offlineMode.enabled` gate always runs first; overriding doesn't bypass an explicit offline-mode opt-out. - If the override throws, `isOnline()` logs and falls through to the platform probe so a buggy callback can't brick sync. Production callers should leave this null. Legitimate use is the emulator+adb-reverse dev workflow, where the consumer sets `isOnlineOverride: () async => true`. The existing private `_isOnlineOverrideForTesting` test seam is unchanged. Both `FrappeSDK()` and `FrappeSDK.forTesting()` accept the parameter; the test constructor initialises it to null in its initializer list. 183 sync-related tests pass unchanged on this branch.
Omprakash-48
approved these changes
Jun 2, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Adds an optional
Future<bool> Function()? isOnlineOverrideparameter onSyncServiceandFrappeSDK. When set, the override replaces theconnectivity_plusprobe insideSyncService.isOnline(). When unset (the default), behaviour is unchanged.Why this is needed
SyncService.isOnline()callsConnectivity().checkConnectivity()directly and only acceptsmobile | wifi | ethernet:On Android emulators with
adb reverse, the platform reportsConnectivityResult.nonebecause the tunnel isn't a recognised network type — even though HTTP traffic to the bench flows fine.pullSync/pullSyncMany/pushSyncthen early-exit withnoConnectivityand refuse to run.Today consumers have no app-side escape:
ConnectivityWatcherisn't exported from the barrel.SyncService.isOnline()bypasses the watcher anyway — it probes the plugin directly.FrappeSDKconstructor takes no hook to replace the probe.The only workaround until now has been to bypass
SyncService.pullSync/pullSyncManyentirely from the consumer app — losing the SDK's delta cursors, page lookahead, and RESUME-on-crash semantics in the process. We were carrying that bypass for ~2 weeks in the Swasti V3 mobile app before this PR.What this PR changes
SyncServiceconstructor acceptsFuture<bool> Function()? isOnlineOverride.SyncService.isOnline()calls the override first when set; falls back to the platform probe on throw (with a logged warning) so a buggy override can't brick sync.FrappeSDK()acceptsisOnlineOverrideand threads it through toSyncService.FrappeSDK.forTestingconstructor initialises the field tonullin its initializer list to keep tests source-compatible.Backward compatibility
null— existingFrappeSDK(baseUrl: ...)andSyncService(...)call sites behave exactly as before._isOnlineOverrideForTestingtest seam infrappe_sdk.dartis unchanged.Safety notes
offlineMode.enabledgate runs first; the override does not bypass an explicit offline-mode opt-out.isOnline()logs the failure and falls through to the platform probe.Intended use
Dev builds that talk to a local bench via
adb reverse:Production callers should leave
isOnlineOverridenull and rely on the platform probe.Tests
183 sync-related tests pass unchanged on this branch (
test/sync/,test/concurrency/sync_mutex_test.dart,test/services/sync_engine_builder_send_test.dart). No new tests added — the change is opt-in and the existing suite covers the default path.