Skip to content

Commit fafcd59

Browse files
KyleAMathewsclaude
andcommitted
fix(client): publish synthetic must-refetch control message on 409
The previous commit removed the raw 409 body publish entirely, but Shape relies on the must-refetch control message to clear its accumulated data and trigger snapshot re-execution. Instead of publishing the raw response body (which could contain stale data rows), publish a synthetic control-only message. Also: fix flaky SSE fallback test (remove brittle upper bound on SSE request count), refine error-path-publish rule to allow static array literal arguments (synthetic control messages) while still catching dynamic publishes from error data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent df690e9 commit fafcd59

File tree

4 files changed

+31
-8
lines changed

4 files changed

+31
-8
lines changed

packages/typescript-client/bin/lib/shape-stream-static-analysis.mjs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1022,6 +1022,7 @@ function buildErrorPathPublishReport(sourceFile, classDecl) {
10221022
if (!ts.isCallExpression(inner)) return
10231023
const callee = getThisMemberName(inner.expression)
10241024
if (!callee || !SUBSCRIBER_METHODS.has(callee)) return
1025+
if (isStaticControlMessagePublish(inner)) return
10251026

10261027
report.push({
10271028
method: methodName,
@@ -1041,6 +1042,7 @@ function buildErrorPathPublishReport(sourceFile, classDecl) {
10411042
if (!ts.isCallExpression(inner)) return
10421043
const callee = getThisMemberName(inner.expression)
10431044
if (!callee || !SUBSCRIBER_METHODS.has(callee)) return
1045+
if (isStaticControlMessagePublish(inner)) return
10441046

10451047
report.push({
10461048
method: methodName,
@@ -1058,6 +1060,18 @@ function buildErrorPathPublishReport(sourceFile, classDecl) {
10581060
return report.sort(compareReports)
10591061
}
10601062

1063+
/**
1064+
* Returns true if a call expression is a #publish([{ headers: { control: ... } }])
1065+
* with a static array literal containing only object literals. These synthetic
1066+
* control-only publishes are intentional (e.g., the must-refetch notification
1067+
* in the 409 handler) and should not trigger the error-path-publish rule.
1068+
*/
1069+
function isStaticControlMessagePublish(callExpr) {
1070+
const [firstArg] = callExpr.arguments
1071+
if (!firstArg || !ts.isArrayLiteralExpression(firstArg)) return false
1072+
return firstArg.elements.every((el) => ts.isObjectLiteralExpression(el))
1073+
}
1074+
10611075
/**
10621076
* Returns true if the expression checks an HTTP error status (4xx or 5xx).
10631077
* Recurses into && and || expressions.

packages/typescript-client/src/client.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -920,6 +920,12 @@ export class ShapeStream<T extends Row<unknown> = Row>
920920
const nextRequestShapeCacheBuster = createCacheBuster()
921921
this.#reset(newShapeHandle)
922922

923+
// Notify subscribers that data must be re-fetched so they can
924+
// clear accumulated state (e.g., Shape clears its row map).
925+
// We publish a synthetic control message rather than the raw 409
926+
// body to avoid delivering stale data rows to subscribers.
927+
await this.#publish([{ headers: { control: `must-refetch` } }])
928+
923929
return this.#requestShape(nextRequestShapeCacheBuster)
924930
} else {
925931
// errors that have reached this point are not actionable without

packages/typescript-client/test/client.test.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2987,11 +2987,10 @@ it(
29872987
return urlObj.searchParams.get(`live_sse`) === `true`
29882988
})
29892989

2990-
// After fallback, should see 3-5 SSE requests (3 short ones trigger fallback,
2991-
// but there may be more in flight due to async timing between the
2992-
// fallback decision and requests already dispatched)
2990+
// At least 3 SSE requests should have been made before fallback triggered.
2991+
// The exact upper bound is non-deterministic due to async timing between
2992+
// the fallback decision and requests already dispatched.
29932993
expect(allSseRequests.length).toBeGreaterThanOrEqual(3)
2994-
expect(allSseRequests.length).toBeLessThanOrEqual(5)
29952994

29962995
unsubscribe()
29972996
} finally {

packages/typescript-client/test/model-based.test.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -559,7 +559,7 @@ class Respond409SameHandleCmd
559559
// ─── Scenario Tests ────────────────────────────────────────────────
560560

561561
describe(`ShapeStream targeted scenario tests`, () => {
562-
it(`409 does not publish messages to subscribers`, async () => {
562+
it(`409 publishes only a synthetic must-refetch control message`, async () => {
563563
resetSharedState()
564564

565565
const initialHandle = `test-handle`
@@ -593,13 +593,17 @@ describe(`ShapeStream targeted scenario tests`, () => {
593593

594594
receivedBatches.length = 0
595595

596-
// Send a 409 with a JSON body — the must-refetch control message
596+
// Send a 409 — client resets and publishes a synthetic must-refetch
597597
await gate.waitForRequest()
598598
gate.provideResponse(make409(`new-handle`))
599599
await waitUntilSettled(gate, errorRef)
600600

601-
// Subscriber should NOT have received any messages from the 409
602-
expect(receivedBatches).toEqual([])
601+
// Subscriber should receive only the synthetic must-refetch control
602+
// message — no data rows from the raw 409 response body
603+
expect(receivedBatches).toHaveLength(1)
604+
expect(receivedBatches[0]).toEqual([
605+
{ headers: { control: `must-refetch` } },
606+
])
603607

604608
aborter.abort()
605609
})

0 commit comments

Comments
 (0)