Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 77 additions & 3 deletions packages/client-runtime/src/connection/supervisor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,7 +488,7 @@ describe("EnvironmentSupervisor", () => {
}).pipe(Effect.provide(TestClock.layer())),
);

it.effect("retries when a session never becomes ready", () =>
it.effect("retries when a session never becomes ready within the snapshot bound", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
ready: () => Effect.never,
Expand All @@ -501,8 +501,14 @@ describe("EnvironmentSupervisor", () => {
supervisor.state,
(state) => state.phase === "connecting" && state.stage === "synchronizing",
);
yield* TestClock.adjust("14 seconds");
expect((yield* SubscriptionRef.get(supervisor.state)).stage).toBe("synchronizing");
// The setup budget has passed, but a synchronizing session keeps going.
yield* TestClock.adjust("15 seconds");
yield* TestClock.adjust("119 seconds");
expect(yield* SubscriptionRef.get(supervisor.state)).toMatchObject({
phase: "connecting",
stage: "synchronizing",
});
expect(yield* Ref.get(harness.releaseCount)).toBe(0);

yield* TestClock.adjust("1 second");
const retrying = yield* awaitState(supervisor.state, (state) => state.phase === "backoff");
Expand All @@ -520,6 +526,74 @@ describe("EnvironmentSupervisor", () => {
}).pipe(Effect.provide(TestClock.layer())),
);

it.effect(
"keeps a working connection while the first snapshot takes longer than the setup budget",
() =>
Effect.gen(function* () {
const harness = yield* makeHarness({
ready: () => Effect.sleep("40 seconds"),
});
const supervisor = yield* EnvironmentSupervisor.make(TARGET_ENTRY, {
initiallyDesired: true,
}).pipe(Effect.provide(harness.dependencies));

yield* awaitState(
supervisor.state,
(state) => state.phase === "connecting" && state.stage === "synchronizing",
);
yield* TestClock.adjust("20 seconds");
expect((yield* SubscriptionRef.get(supervisor.state)).phase).toBe("connecting");

yield* TestClock.adjust("20 seconds");
const connected = yield* awaitState(
supervisor.state,
(state) => state.phase === "connected",
);

expect(connected.attempt).toBe(1);
expect(yield* Ref.get(harness.sessionCount)).toBe(1);
expect(yield* Ref.get(harness.releaseCount)).toBe(0);
}).pipe(Effect.provide(TestClock.layer())),
);

it.effect(
"reconnects as soon as a synchronizing session fails instead of waiting out the bound",
() =>
Effect.gen(function* () {
const harness = yield* makeHarness({
ready: (attempt) =>
attempt === 1
? Effect.sleep("20 seconds").pipe(
Effect.andThen(
Effect.fail(
new ConnectionTransientError({
reason: "transport",
detail: "Test environment stopped responding.",
}),
),
),
)
: Effect.void,
});
const supervisor = yield* EnvironmentSupervisor.make(TARGET_ENTRY, {
initiallyDesired: true,
}).pipe(Effect.provide(harness.dependencies));

yield* awaitState(
supervisor.state,
(state) => state.phase === "connecting" && state.stage === "synchronizing",
);
yield* TestClock.adjust("20 seconds");
const retrying = yield* awaitState(supervisor.state, (state) => state.phase === "backoff");

expect(retrying.lastFailure).toMatchObject({
reason: "transport",
message: "Test environment stopped responding.",
});
expect(yield* Ref.get(harness.releaseCount)).toBe(1);
}).pipe(Effect.provide(TestClock.layer())),
);

it.effect("interrupts and releases a connection attempt when setup times out", () =>
Effect.gen(function* () {
const harness = yield* makeHarness({
Expand Down
23 changes: 21 additions & 2 deletions packages/client-runtime/src/connection/supervisor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@ const RETRY_BASE_DELAY_MS = 1_000;
const RETRY_MAX_DELAY_MS = 300_000;
const CONNECTION_ESTABLISHMENT_TIMEOUT = "15 seconds";
const establishmentTimeout = Duration.fromInputUnsafe(CONNECTION_ESTABLISHMENT_TIMEOUT);
// Extra time for the first config snapshot once the session is synchronizing.
// A server that is busy building it must not lose a socket that is working, and
// a dead socket never gets this far: the open timeout and the ping/pong check
// fail the session first. This is only the hard bound for a handler that stays
// silent behind a live connection.
const FIRST_SNAPSHOT_TIMEOUT = "2 minutes";
const firstSnapshotTimeout = Duration.fromInputUnsafe(FIRST_SNAPSHOT_TIMEOUT);
const CONNECTION_PROBE_TIMEOUT = "15 seconds";
// Mobile resumes, explicit retries, and offline events want a fast answer:
// the user is waiting, or the network may be gone.
Expand Down Expand Up @@ -274,6 +281,9 @@ export const make = Effect.fn("EnvironmentSupervisor.make")(function* (
const intent = yield* Ref.make(initialIntent);
const signals = yield* Queue.unbounded<SupervisorSignal>();
const resetRetryState = yield* Ref.make(false);
// Whether the running attempt has a session and is waiting for its first
// config snapshot. Reset by the next route's earlier stages.
const synchronizing = yield* Ref.make(false);
// Set while a probe of the live session is running, and kept when it fails
// or times out: something asked whether the connection still works and it
// closed or failed before answering, so the follow-up reconnect skips the
Expand Down Expand Up @@ -414,6 +424,7 @@ export const make = Effect.fn("EnvironmentSupervisor.make")(function* (
if ("prepared" in progress) {
yield* SubscriptionRef.set(prepared, Option.some(progress.prepared));
}
yield* Ref.set(synchronizing, progress.stage === "synchronizing");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 High connection/supervisor.ts:427

A setup attempt can wait an extra firstSnapshotTimeout and delay retry even though its current route is still preparing credentials. synchronizing remains true if an earlier route reached that stage and its session failed, because driver.ts reports preparing only once before trying routes; reset the flag when each route begins preparation.

🤖 Copy this AI Prompt to have your agent fix this:
In file @packages/client-runtime/src/connection/supervisor.ts around line 427:

A setup attempt can wait an extra `firstSnapshotTimeout` and delay retry even though its current route is still preparing credentials. `synchronizing` remains true if an earlier route reached that stage and its session failed, because `driver.ts` reports `preparing` only once before trying routes; reset the flag when each route begins preparation.

yield* setState(
connectingState(yield* Ref.get(intent), generation, attempt, lastFailure, progress.stage),
);
Expand Down Expand Up @@ -694,10 +705,18 @@ export const make = Effect.fn("EnvironmentSupervisor.make")(function* (
resetRetry,
})),
),
// Each route may use the full setup time before the next is tried.
// Each route may use the full setup time before the next is tried. An
// attempt that spent it on credentials, the session or the socket fails
// then; one already waiting for the first snapshot gets a longer bound.
Effect.sleep(
Duration.times(establishmentTimeout, connectionRoutes(yield* Ref.get(currentEntry)).length),
).pipe(Effect.as<EstablishmentEvent>({ _tag: "TimedOut" })),
).pipe(
Effect.andThen(Ref.get(synchronizing)),
Effect.flatMap((waitingForSnapshot) =>
waitingForSnapshot ? Effect.sleep(firstSnapshotTimeout) : Effect.void,
),
Effect.as<EstablishmentEvent>({ _tag: "TimedOut" }),
),
]);

if (establishment._tag === "Interrupted") {
Expand Down
Loading