-
Notifications
You must be signed in to change notification settings - Fork 3.7k
fix(mobile): reconnect faster and stop resyncing more than needed after backgrounding #5154
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: main
Are you sure you want to change the base?
Changes from all commits
4abafa4
4df84c9
2c8fe66
b889ec4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import { EnvironmentId } from "@t3tools/contracts"; | ||
| import { RelayClientTracer } from "@t3tools/shared/relayTracing"; | ||
| import { describe, expect, it } from "@effect/vitest"; | ||
| import * as Clock from "effect/Clock"; | ||
| import * as Deferred from "effect/Deferred"; | ||
| import * as Effect from "effect/Effect"; | ||
| import * as Layer from "effect/Layer"; | ||
|
|
@@ -358,7 +359,16 @@ describe("EnvironmentSupervisor", () => { | |
| ); | ||
| expect(yield* Ref.get(harness.prepareCount)).toBe(1); | ||
|
|
||
| for (const [index, delay] of [1_000, 2_000, 4_000, 8_000, 16_000, 16_000].entries()) { | ||
| // Jitter puts each delay in [rung/2, rung]. Read the scheduled retryAt | ||
| // per rung, assert it falls inside the jitter bounds (proving ladder | ||
| // progression and the 16s cap), then advance exactly to it. | ||
| for (const [index, rung] of [1_000, 2_000, 4_000, 8_000, 16_000, 16_000].entries()) { | ||
| const current = yield* SubscriptionRef.get(supervisor.state); | ||
| const now = yield* Clock.currentTimeMillis; | ||
| expect(current.retryAt).not.toBeNull(); | ||
| const delay = (current.retryAt ?? now) - now; | ||
| expect(delay).toBeGreaterThanOrEqual(rung / 2); | ||
| expect(delay).toBeLessThanOrEqual(rung); | ||
| yield* TestClock.adjust(delay); | ||
| yield* eventuallyState( | ||
| supervisor.state, | ||
|
|
@@ -539,9 +549,10 @@ describe("EnvironmentSupervisor", () => { | |
| ); | ||
| expect(yield* Ref.get(harness.prepareCount)).toBe(3); | ||
|
|
||
| yield* TestClock.adjust("999 millis"); | ||
| // Jittered first-rung delay lands in [500ms, 1000ms]. | ||
| yield* TestClock.adjust("499 millis"); | ||
| expect(yield* Ref.get(harness.prepareCount)).toBe(3); | ||
| yield* TestClock.adjust("1 milli"); | ||
| yield* TestClock.adjust("501 millis"); | ||
| yield* eventuallyState( | ||
| supervisor.state, | ||
| (state) => state.phase === "backoff" && state.attempt === 2, | ||
|
|
@@ -873,6 +884,82 @@ describe("EnvironmentSupervisor", () => { | |
| }), | ||
| ); | ||
|
|
||
| it.effect("probes the active session when the network path changes", () => | ||
| Effect.gen(function* () { | ||
| const probeCount = yield* Ref.make(0); | ||
| const probeCalled = yield* Deferred.make<void>(); | ||
| const harness = yield* makeHarness({ | ||
| probe: () => | ||
| Ref.update(probeCount, (count) => count + 1).pipe( | ||
| Effect.andThen(Deferred.succeed(probeCalled, undefined)), | ||
| ), | ||
| }); | ||
| const supervisor = yield* EnvironmentSupervisor.make(TARGET_ENTRY, { | ||
| initiallyDesired: true, | ||
| }).pipe(Effect.provide(harness.dependencies)); | ||
|
|
||
| yield* awaitState(supervisor.state, (state) => state.phase === "connected"); | ||
| yield* harness.wake("network-path-changed"); | ||
| yield* Deferred.await(probeCalled); | ||
|
|
||
| expect(yield* Ref.get(probeCount)).toBe(1); | ||
| expect(yield* Ref.get(harness.sessionCount)).toBe(1); | ||
| expect(yield* Ref.get(harness.releaseCount)).toBe(0); | ||
| expect((yield* SubscriptionRef.get(supervisor.state)).phase).toBe("connected"); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("does not cut backoff short when the network path flaps", () => | ||
| Effect.gen(function* () { | ||
| const harness = yield* makeHarness({ | ||
| prepare: () => Effect.fail(transient()), | ||
| }); | ||
| const supervisor = yield* EnvironmentSupervisor.make(TARGET_ENTRY, { | ||
| initiallyDesired: true, | ||
| }).pipe(Effect.provide(harness.dependencies)); | ||
|
|
||
| yield* awaitState( | ||
| supervisor.state, | ||
| (state) => state.phase === "backoff" && state.attempt === 1, | ||
| ); | ||
| expect(yield* Ref.get(harness.prepareCount)).toBe(1); | ||
|
|
||
| // Advisory path-change wakeups have no session to probe during backoff | ||
| // and must not trigger an early retry. | ||
| yield* harness.wake("network-path-changed"); | ||
| yield* harness.wake("network-path-changed"); | ||
| for (let attempt = 0; attempt < 20; attempt += 1) { | ||
| yield* Effect.yieldNow; | ||
| } | ||
| expect(yield* Ref.get(harness.prepareCount)).toBe(1); | ||
| expect((yield* SubscriptionRef.get(supervisor.state)).phase).toBe("backoff"); | ||
| }), | ||
| ); | ||
|
Comment on lines
+912
to
+937
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Replace the fixed This test waits for two queued "network-path-changed" wakeups to be processed by looping Use a typed receipt instead, for example by extending the harness to expose an effect that completes once the As per coding guidelines, "Tests must wait for typed receipts and worker drains in event-sourced async flows; do not use sleeps, polling, or arbitrary timeouts to make tests pass." 🤖 Prompt for AI AgentsSource: Coding guidelines |
||
|
|
||
| it.effect("replaces the session when a probe after a network path change fails", () => | ||
| Effect.gen(function* () { | ||
| const harness = yield* makeHarness({ | ||
| probe: (attempt) => | ||
| attempt === 1 | ||
| ? Effect.fail(transient("The path changed under the socket.")) | ||
| : Effect.void, | ||
| }); | ||
| const supervisor = yield* EnvironmentSupervisor.make(TARGET_ENTRY, { | ||
| initiallyDesired: true, | ||
| }).pipe(Effect.provide(harness.dependencies)); | ||
|
|
||
| yield* awaitState(supervisor.state, (state) => state.phase === "connected"); | ||
| yield* harness.wake("network-path-changed"); | ||
| yield* awaitState( | ||
| supervisor.state, | ||
| (state) => state.phase === "connected" && state.generation === 2, | ||
| ); | ||
|
|
||
| expect(yield* Ref.get(harness.sessionCount)).toBe(2); | ||
| expect(yield* Ref.get(harness.releaseCount)).toBe(1); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("immediately replaces a mobile session after a long background resume", () => | ||
| Effect.gen(function* () { | ||
| const probeCount = yield* Ref.make(0); | ||
|
|
@@ -937,9 +1024,9 @@ describe("EnvironmentSupervisor", () => { | |
|
|
||
| yield* awaitState(supervisor.state, (state) => state.phase === "connected"); | ||
| yield* harness.wake("application-active"); | ||
| yield* awaitState(supervisor.state, (state) => state.phase === "backoff"); | ||
| yield* TestClock.adjust("1 second"); | ||
| yield* eventuallyState( | ||
| // A failed probe already proves the session is dead: the supervisor | ||
| // replaces it immediately without entering backoff. | ||
| yield* awaitState( | ||
| supervisor.state, | ||
| (state) => state.phase === "connected" && state.generation === 2, | ||
| ); | ||
|
|
@@ -949,6 +1036,34 @@ describe("EnvironmentSupervisor", () => { | |
| }).pipe(Effect.provide(TestClock.layer())), | ||
| ); | ||
|
|
||
| it.effect("parks in blocked when the foreground probe fails with a blocked error", () => | ||
| Effect.gen(function* () { | ||
| const harness = yield* makeHarness({ | ||
| probe: (attempt) => (attempt === 1 ? Effect.fail(blocked()) : Effect.void), | ||
| }); | ||
| const supervisor = yield* EnvironmentSupervisor.make(TARGET_ENTRY, { | ||
| initiallyDesired: true, | ||
| }).pipe(Effect.provide(harness.dependencies)); | ||
|
|
||
| yield* awaitState(supervisor.state, (state) => state.phase === "connected"); | ||
| yield* harness.wake("application-active"); | ||
| // A blocked probe failure (auth revoked, permissions) must not churn | ||
| // immediate reconnects; the supervisor parks until an external signal. | ||
| yield* awaitState( | ||
| supervisor.state, | ||
| (state) => state.phase === "blocked" && state.lastFailure?.reason === "authentication", | ||
| ); | ||
| expect(yield* Ref.get(harness.sessionCount)).toBe(1); | ||
| expect(yield* Ref.get(harness.releaseCount)).toBe(1); | ||
|
|
||
| yield* harness.wake("application-active"); | ||
| yield* awaitState( | ||
| supervisor.state, | ||
| (state) => state.phase === "connected" && state.generation === 2, | ||
| ); | ||
| }), | ||
| ); | ||
|
|
||
| it.effect("quickly times out a stalled mobile foreground liveness probe", () => | ||
| Effect.gen(function* () { | ||
| const harness = yield* makeHarness({ | ||
|
|
@@ -961,11 +1076,8 @@ describe("EnvironmentSupervisor", () => { | |
| yield* awaitState(supervisor.state, (state) => state.phase === "connected"); | ||
| yield* harness.wake("application-active-probe"); | ||
| yield* TestClock.adjust("3 seconds"); | ||
| yield* awaitState( | ||
| supervisor.state, | ||
| (state) => state.phase === "backoff" && state.lastFailure?.reason === "timeout", | ||
| ); | ||
| yield* TestClock.adjust("1 second"); | ||
| // The 3s mobile probe timeout counts as a failed probe: the stale | ||
| // session is replaced immediately without a backoff delay. | ||
| yield* eventuallyState( | ||
| supervisor.state, | ||
| (state) => state.phase === "connected" && state.generation === 2, | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.