From 79888cfc56a1b5493692b883a4bbbce752f1ddae Mon Sep 17 00:00:00 2001 From: Jimmy Song Date: Mon, 18 May 2026 11:44:32 -0700 Subject: [PATCH] fix(reconcile): always run reconcileSubscription after a fetch attempt Reactive getConfig() switching subscribe to undefined on an error response (e.g. poll only while response.ok) silently failed when the error body did not match the result entity shape. runQuery only called reconcileSubscription after applyData succeeded, so parseEntities throwing on the body shape skipped the reconcile, the inner reactiveSignal never re-read this.config, and the running poll subscriber kept ticking against stale config. Move the call into a finally block so it fires after every fetch attempt. On adapter-level failures where ctx.response was never updated this is a no-op: the cached config is returned and the ref check inside reconcileSubscription short-circuits. The existing poll.test "stops polling when getConfig() switches subscribe to undefined after an error" is rewritten to use t.entity() with an unparseable 404 body, which fails on main and passes with this change. Co-Authored-By: Claude Opus 4.7 (1M context) --- .changeset/reconcile-on-applydata-throw.md | 5 +++ packages/fetchium/src/QueryResult.ts | 22 ++++++++------ packages/fetchium/src/__tests__/poll.test.ts | 32 +++++++++++++++++--- 3 files changed, 45 insertions(+), 14 deletions(-) create mode 100644 .changeset/reconcile-on-applydata-throw.md diff --git a/.changeset/reconcile-on-applydata-throw.md b/.changeset/reconcile-on-applydata-throw.md new file mode 100644 index 0000000..4248dcd --- /dev/null +++ b/.changeset/reconcile-on-applydata-throw.md @@ -0,0 +1,5 @@ +--- +'fetchium': patch +--- + +Fix reactive `getConfig()` not reacting to error responses when the response body fails to parse against the result schema. Previously `runQuery` only called `reconcileSubscription` after `applyData` succeeded, so a 404 (or any other status) whose body did not match the entity shape would throw inside `parseEntities`, skip the reconcile, and leave the running subscriber installed against stale config. The reconcile call is now in a `finally` block so it fires after every fetch attempt, regardless of whether parsing succeeds. diff --git a/packages/fetchium/src/QueryResult.ts b/packages/fetchium/src/QueryResult.ts index ce8cf38..d7d6e5a 100644 --- a/packages/fetchium/src/QueryResult.ts +++ b/packages/fetchium/src/QueryResult.ts @@ -328,15 +328,19 @@ export class QueryInstance { return withRetry( async () => { - const freshData = await adapter.send(ctx, signal); - this.updatedAt = Date.now(); - - const result = this.applyData(freshData, true); - this.saveQueryMetadata(); - - this.reconcileSubscription(); - - return result; + try { + const freshData = await adapter.send(ctx, signal); + this.updatedAt = Date.now(); + + const result = this.applyData(freshData, true); + this.saveQueryMetadata(); + + return result; + } finally { + // In finally so reactive getConfig() reacts to error responses + // (e.g. 404 → subscribe: undefined) even when applyData throws. + this.reconcileSubscription(); + } }, this.retryConfig, signal, diff --git a/packages/fetchium/src/__tests__/poll.test.ts b/packages/fetchium/src/__tests__/poll.test.ts index 39aa5fb..aeedbee 100644 --- a/packages/fetchium/src/__tests__/poll.test.ts +++ b/packages/fetchium/src/__tests__/poll.test.ts @@ -2,6 +2,7 @@ import { describe, it, expect } from 'vitest'; import { reactiveSignal } from 'signalium'; import { RESTQuery } from '../rest/index.js'; import { fetchQuery } from '../query.js'; +import { Entity } from '../proxy.js'; import { testWithClient, sleep, setupTestClient } from './utils.js'; import { t } from '../typeDefs.js'; import { poll } from '../subscriptions/polling.js'; @@ -175,14 +176,34 @@ describe('poll() factory', () => { it('stops polling when getConfig() switches subscribe to undefined after an error', async () => { const { client, mockFetch } = getClient(); let callCount = 0; - // First call: 200 OK. - mockFetch.get('/maybe-gone', () => ({ n: ++callCount })); - // Subsequent calls: 404. - mockFetch.get('/maybe-gone', () => ({ n: ++callCount }), { status: 404 }); + class Item extends Entity { + __typename = t.typename('PollStopItem'); + id = t.id; + name = t.string; + } + + // First call: 200 OK with valid entity body so the poll subscriber installs. + mockFetch.get('/maybe-gone', () => { + callCount++; + return { __typename: 'PollStopItem', id: '1', name: 'ok' }; + }); + // Subsequent calls: 404 with an error body that does NOT match the entity + // shape, so applyData throws via parseEntities. reconcileSubscription + // must still fire so the reactive getConfig sees the response transition. + mockFetch.get( + '/maybe-gone', + () => { + callCount++; + return { error: 'Not found' }; + }, + { status: 404 }, + ); class GetMaybeGone extends RESTQuery { path = '/maybe-gone'; - result = { n: t.number }; + result = t.entity(Item); + + config = { retry: { retries: 0 } as const }; getConfig() { const is404 = reactiveSignal(() => { @@ -191,6 +212,7 @@ describe('poll() factory', () => { }).value; return { + ...this.config, subscribe: is404 ? undefined : poll({ interval: 100 }), }; }