Skip to content

Commit fe5b51e

Browse files
authored
fix(server): an interrupted provider update ends failed instead of running forever (pingdotgg#33975)
1 parent 0c6b5af commit fe5b51e

2 files changed

Lines changed: 83 additions & 2 deletions

File tree

‎apps/server/src/provider/providerMaintenanceRunner.test.ts‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -893,6 +893,66 @@ describe("providerMaintenanceRunner", () => {
893893
);
894894
});
895895

896+
it.effect("an interrupted update ends failed instead of running forever", () => {
897+
const started = Deferred.makeUnsafe<void>();
898+
return Effect.gen(function* () {
899+
const { registry, updateStatesRef } = yield* makeRegistry(baseNativeCliProvider);
900+
const updater = yield* makeTestRunner(registry);
901+
const update = yield* updater.updateProvider(NATIVE_CLI_DRIVER).pipe(Effect.forkScoped);
902+
yield* Deferred.await(started);
903+
yield* Fiber.interrupt(update);
904+
const last = (yield* Ref.get(updateStatesRef)).at(-1);
905+
assert.deepStrictEqual(
906+
[last?.status, last?.message],
907+
["failed", "Update stopped before it finished."],
908+
);
909+
}).pipe(
910+
Effect.provide(
911+
Layer.mergeAll(
912+
layerNonWindowsPlatform,
913+
layerLatestVersionHttpClient("1.0.0"),
914+
layerMockSpawner(() => ({
915+
exitCode: Deferred.succeed(started, undefined).pipe(Effect.andThen(Effect.never)),
916+
})),
917+
),
918+
),
919+
);
920+
});
921+
922+
it.effect("a request dropped while the final state is written still records it", () => {
923+
const writing = Deferred.makeUnsafe<void>();
924+
const release = Deferred.makeUnsafe<void>();
925+
return Effect.gen(function* () {
926+
const { registry, updateStatesRef } = yield* makeRegistry(baseNativeCliProvider);
927+
const updater = yield* makeTestRunner({
928+
...registry,
929+
setProviderMaintenanceActionState: (input) =>
930+
input.state?.status === "succeeded"
931+
? Deferred.succeed(writing, undefined).pipe(
932+
Effect.andThen(Deferred.await(release)),
933+
Effect.andThen(registry.setProviderMaintenanceActionState(input)),
934+
)
935+
: registry.setProviderMaintenanceActionState(input),
936+
});
937+
938+
const update = yield* updater.updateProvider(NATIVE_CLI_DRIVER).pipe(Effect.forkScoped);
939+
yield* Deferred.await(writing);
940+
update.interruptUnsafe();
941+
yield* Deferred.succeed(release, undefined);
942+
yield* Fiber.await(update);
943+
944+
assert.strictEqual((yield* Ref.get(updateStatesRef)).at(-1)?.status, "succeeded");
945+
}).pipe(
946+
Effect.provide(
947+
Layer.mergeAll(
948+
layerNonWindowsPlatform,
949+
layerLatestVersionHttpClient("1.0.0"),
950+
layerMockSpawner(() => ({ stdout: "updated" })),
951+
),
952+
),
953+
);
954+
});
955+
896956
it.effect("prevents concurrent updates for the same provider", () => {
897957
const startedLatch: { resolve: () => void } = { resolve: () => {} };
898958
const releaseLatch: { resolve: () => void } = { resolve: () => {} };

‎apps/server/src/provider/providerMaintenanceRunner.ts‎

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,8 @@ export const make = Effect.fn("ProviderMaintenanceRunner.make")(function* () {
419419
});
420420
}
421421

422+
const startedAtRef = yield* Ref.make<string | null>(null);
423+
const settled = yield* Ref.make(false);
422424
const setUpdateState = (state: ServerProviderUpdateState | null) =>
423425
providerRegistry.setProviderMaintenanceActionState({
424426
instanceId,
@@ -436,9 +438,13 @@ export const make = Effect.fn("ProviderMaintenanceRunner.make")(function* () {
436438

437439
const runProviderUpdate = Effect.fn("ProviderMaintenanceRunner.runProviderUpdate")(
438440
function* () {
441+
// Uninterruptible, so a dropped request cannot leave settled set without the state.
439442
const finish = (state: ServerProviderUpdateState) =>
440-
setUpdateState(state).pipe(Effect.map((providers) => ({ providers })));
441-
const startedAtRef = yield* Ref.make<string | null>(null);
443+
Ref.set(settled, true).pipe(
444+
Effect.andThen(setUpdateState(state)),
445+
Effect.map((providers) => ({ providers })),
446+
Effect.uninterruptible,
447+
);
442448

443449
const runCommandAndVerify = Effect.fn("ProviderMaintenanceRunner.runCommandAndVerify")(
444450
function* () {
@@ -655,6 +661,21 @@ export const make = Effect.fn("ProviderMaintenanceRunner.make")(function* () {
655661
run: runProviderUpdate(),
656662
})
657663
.pipe(
664+
// A dropped request interrupts the update; a "running" state would spin until restart.
665+
Effect.onInterrupt(() =>
666+
Effect.gen(function* () {
667+
// A finished state stays, even if the request drops while it is being written.
668+
if (yield* Ref.get(settled)) return;
669+
yield* setUpdateState(
670+
makeUpdateState({
671+
status: "failed",
672+
startedAt: yield* Ref.get(startedAtRef),
673+
finishedAt: yield* nowIso,
674+
message: "Update stopped before it finished.",
675+
}),
676+
);
677+
}).pipe(Effect.ignore),
678+
),
658679
Effect.mapError((error) =>
659680
isServerProviderUpdateError(error)
660681
? new ServerProviderUpdateError({

0 commit comments

Comments
 (0)