fix: v0.17.0 — sim-control POST retries + permission-aware notifications e2e
CI / build + unit + e2e (pull_request) Successful in 6m52s
CI / build + unit + e2e (pull_request) Successful in 6m52s
This commit is contained in:
+62
-16
@@ -48,16 +48,52 @@ export function pendingScaleConfirmed(pendingScale: number | null, serverTimeSca
|
||||
return pendingScale !== null && serverTimeScale === pendingScale;
|
||||
}
|
||||
|
||||
// ISS-008 fix (v0.17.0, CI runs #44/#47): bounded retry for POST /api/sim — a commit dropped by
|
||||
// the runner's flaky uplink used to leave the UI optimistic while the server kept the old scale.
|
||||
// Up to maxAttempts tries with delayMs between them; stops early once superseded() flips (a newer
|
||||
// intent arrived and the caller's loop sends it instead). post is idempotent set-semantics, so
|
||||
// re-sending a body whose response was lost cannot double-apply. Pure core — unit-tested in node.
|
||||
export const SIM_POST_MAX_ATTEMPTS = 3;
|
||||
export const SIM_POST_RETRY_DELAY_MS = 500;
|
||||
|
||||
export type SimPostRetryResult =
|
||||
| { ok: true }
|
||||
| { ok: false; reason: 'exhausted'; error: unknown }
|
||||
| { ok: false; reason: 'superseded' };
|
||||
|
||||
export async function retrySimPost(
|
||||
post: () => Promise<void>,
|
||||
superseded: () => boolean,
|
||||
sleep: (ms: number) => Promise<void> = (ms) => new Promise((r) => setTimeout(r, ms)),
|
||||
maxAttempts: number = SIM_POST_MAX_ATTEMPTS,
|
||||
delayMs: number = SIM_POST_RETRY_DELAY_MS,
|
||||
): Promise<SimPostRetryResult> {
|
||||
let lastError: unknown;
|
||||
for (let attempt = 1; attempt <= maxAttempts; attempt++) {
|
||||
if (superseded()) return { ok: false, reason: 'superseded' }; // a newer intent takes over — don't even send this body
|
||||
try {
|
||||
await post();
|
||||
return { ok: true };
|
||||
} catch (err) {
|
||||
lastError = err;
|
||||
if (superseded()) return { ok: false, reason: 'superseded' }; // a newer intent arrived during the attempt — don't wait out the backoff
|
||||
if (attempt < maxAttempts) await sleep(delayMs);
|
||||
}
|
||||
}
|
||||
return { ok: false, reason: 'exhausted', error: lastError };
|
||||
}
|
||||
|
||||
export default function LeftPane({ state, loading, error, onRefresh, onHover, onError }: LeftPaneProps) {
|
||||
const [busy, setBusy] = useState(false);
|
||||
|
||||
// POST /api/sim, then refetch the full snapshot immediately so the pane reflects the new
|
||||
// controls without waiting for the next poll tick. On failure keep the stale UI — the
|
||||
// running poll retries on its own and surfaces the error state. A commit that lands while
|
||||
// another POST is in flight is held in a single slot (last intent wins) and flushed by the
|
||||
// in-flight call, so every user intent reaches the server (the [1,1000] clamp stays as backstop).
|
||||
// The gate is a REF, not React state: closures never see a stale "busy" value, which used to
|
||||
// let a flush re-queue instead of send and silently drop a commit.
|
||||
// POST /api/sim (retried up to SIM_POST_MAX_ATTEMPTS× on failure — CI runs #44/#47), then
|
||||
// refetch the full snapshot immediately so the pane reflects the new controls without waiting
|
||||
// for the next poll tick. After the final attempt fails keep the stale UI and surface an error
|
||||
// toast (the running poll retries on its own). A commit that lands while another POST is in
|
||||
// flight — including mid-retry — is held in a single slot (last intent wins) and supersedes
|
||||
// the in-flight body, so every user intent reaches the server (the [1,1000] clamp stays as
|
||||
// backstop). The gate is a REF, not React state: closures never see a stale "busy" value,
|
||||
// which used to let a flush re-queue instead of send and silently drop a commit.
|
||||
const queuedRef = useRef<{ timeScale?: number; paused?: boolean } | null>(null);
|
||||
const busyRef = useRef(false);
|
||||
|
||||
@@ -78,15 +114,25 @@ export default function LeftPane({ state, loading, error, onRefresh, onHover, on
|
||||
try {
|
||||
let current: { timeScale?: number; paused?: boolean } | null = body;
|
||||
while (current !== null) {
|
||||
const res = await fetch('/api/sim', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(current),
|
||||
cache: 'no-store',
|
||||
});
|
||||
if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`);
|
||||
onRefresh();
|
||||
if (current === body) sentOwn = true; // object identity — queued bodies are fresh literals
|
||||
const target = current; // stable across this retry round
|
||||
const result = await retrySimPost(
|
||||
async () => {
|
||||
const res = await fetch('/api/sim', {
|
||||
method: 'POST',
|
||||
headers: { 'Content-Type': 'application/json' },
|
||||
body: JSON.stringify(target),
|
||||
cache: 'no-store',
|
||||
});
|
||||
if (!res.ok) throw new Error(`POST /api/sim → HTTP ${res.status}`);
|
||||
},
|
||||
() => queuedRef.current !== null, // a newer intent supersedes the in-flight body mid-retry
|
||||
);
|
||||
if (result.ok) {
|
||||
onRefresh();
|
||||
if (target === body) sentOwn = true; // object identity — queued bodies are fresh literals
|
||||
} else if (result.reason === 'exhausted') {
|
||||
throw result.error; // only after the final attempt fails → error toast below
|
||||
}
|
||||
current = queuedRef.current; // pick up anything that arrived while we were in flight
|
||||
queuedRef.current = null;
|
||||
}
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from 'vitest';
|
||||
import { pendingScaleConfirmed } from './LeftPane';
|
||||
import { describe, expect, it, vi } from 'vitest';
|
||||
import { pendingScaleConfirmed, retrySimPost, SIM_POST_MAX_ATTEMPTS, SIM_POST_RETRY_DELAY_MS } from './LeftPane';
|
||||
|
||||
// ISS-001 — the optimistic slider value is cleared exactly when a rendered snapshot confirms
|
||||
// it (server scale === pending), never earlier. Clearing at commit time would snap the pointer
|
||||
@@ -22,3 +22,70 @@ describe('pendingScaleConfirmed', () => {
|
||||
expect(pendingScaleConfirmed(1, 1)).toBe(true); // committing the current value confirms immediately
|
||||
});
|
||||
});
|
||||
|
||||
// ISS-008 fix (CI runs #44/#47): a dropped POST /api/sim must be retried before surfacing the
|
||||
// error — up to SIM_POST_MAX_ATTEMPTS tries with SIM_POST_RETRY_DELAY_MS between them, and a newer
|
||||
// intent supersedes the in-flight body instead of waiting out its retries.
|
||||
|
||||
describe('retrySimPost', () => {
|
||||
it('pins the retry budget: 3 attempts total, ~500 ms backoff', () => {
|
||||
expect(SIM_POST_MAX_ATTEMPTS).toBe(3);
|
||||
expect(SIM_POST_RETRY_DELAY_MS).toBe(500);
|
||||
});
|
||||
|
||||
it('succeeds on the first attempt without sleeping or retrying', async () => {
|
||||
const post = vi.fn(async () => {});
|
||||
const sleep = vi.fn(async () => {});
|
||||
await expect(retrySimPost(post, () => false, sleep)).resolves.toEqual({ ok: true });
|
||||
expect(post).toHaveBeenCalledTimes(1);
|
||||
expect(sleep).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('retries with the backoff delay and succeeds on a later attempt', async () => {
|
||||
let calls = 0;
|
||||
const post = vi.fn(async () => {
|
||||
if (++calls < 3) throw new Error('dropped');
|
||||
});
|
||||
const sleep = vi.fn(async () => {});
|
||||
await expect(retrySimPost(post, () => false, sleep)).resolves.toEqual({ ok: true });
|
||||
expect(post).toHaveBeenCalledTimes(3);
|
||||
expect(sleep).toHaveBeenCalledTimes(2); // one backoff between each pair of attempts
|
||||
expect(sleep).toHaveBeenCalledWith(SIM_POST_RETRY_DELAY_MS);
|
||||
});
|
||||
|
||||
it('gives up after the final attempt with the last error (no trailing sleep)', async () => {
|
||||
const err = new Error('down');
|
||||
const post = vi.fn(async () => {
|
||||
throw err;
|
||||
});
|
||||
const sleep = vi.fn(async () => {});
|
||||
const result = await retrySimPost(post, () => false, sleep);
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.reason).toBe('exhausted');
|
||||
if (result.reason === 'exhausted') expect(result.error).toBe(err); // the LAST attempt's error
|
||||
}
|
||||
expect(post).toHaveBeenCalledTimes(SIM_POST_MAX_ATTEMPTS);
|
||||
expect(sleep).toHaveBeenCalledTimes(SIM_POST_MAX_ATTEMPTS - 1);
|
||||
});
|
||||
|
||||
it('stops retrying once a newer intent supersedes the in-flight body (no further attempts or sleeps)', async () => {
|
||||
let calls = 0;
|
||||
const post = vi.fn(async () => {
|
||||
calls++; // flip "superseded" right after the first failure, like a queued commitNow would
|
||||
throw new Error('dropped');
|
||||
});
|
||||
const sleep = vi.fn(async () => {});
|
||||
await expect(retrySimPost(post, () => calls >= 1, sleep)).resolves.toEqual({ ok: false, reason: 'superseded' });
|
||||
expect(post).toHaveBeenCalledTimes(1); // the caller's loop sends the newer body instead
|
||||
expect(sleep).not.toHaveBeenCalled(); // supersede is checked before the backoff sleep
|
||||
});
|
||||
|
||||
it('never even sends a body that is already superseded', async () => {
|
||||
const post = vi.fn(async () => {});
|
||||
const sleep = vi.fn(async () => {});
|
||||
await expect(retrySimPost(post, () => true, sleep)).resolves.toEqual({ ok: false, reason: 'superseded' });
|
||||
expect(post).not.toHaveBeenCalled();
|
||||
expect(sleep).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user