nodes/ComfyAssets-ComfyUI_PromptManager
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(activity): stop the job store on pagehide and let proxied routes outlive teardown

Browse Source 
CI's touch coverage run failed "a resolver failure for the ComfyUI link never
stops the shell from mounting" with "route.fulfill: Fetch response has been
disposed": its trace shows two Library thumbnail requests started by the last
route still in the spec's proxy-mount handler while the page fixture was torn
down.

- The proxy-mount handler ignores only that teardown error, and the workspace
  and Activity specs unroute with ignoreErrors before teardown.
- The Activity store now ends its work on pagehide: the poll and backoff timers
  stop, the job streams close and a list read in flight is aborted (an older
  read is also aborted when a newer one starts). A pageshow from the
  back-forward cache reads the list again.
This commit is contained in:
Vito Sansevero
2026-10-21 11:49:14 -07:00
parent e95ad6da4d
commit 7125681a98
4 changed files with 94 additions and 13 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/browser/workspace-activity.spec.mjs
+2
View File
@@ -68,6 +68,8 @@ test.beforeEach(async ({ page, request, browserName }) => {
});
test.afterEach(async ({ page, request, browserName }) => {
// The store's own reads can meet a route handler at teardown; that must not fail the test it outlived.
await page.unrouteAll({ behavior: 'ignoreErrors' });
await saveCoverage(page, browserName);
expect(page.activityErrors).toEqual([]);
await request.post('/__test__/jobs/restart');
tests/browser/workspace.spec.mjs
+10 -2
View File
@@ -48,6 +48,8 @@ test.beforeEach(async ({ page, request, browserName }) => {
});
test.afterEach(async ({ page, browserName }) => {
// A route handler still running at teardown must not fail the test it outlived.
await page.unrouteAll({ behavior: 'ignoreErrors' });
await saveCoverage(page, browserName);
expect(page.workspaceErrors).toEqual([]);
});
@@ -688,8 +690,14 @@ async function serveUnderProxyMount(page) {
await page.route(url => url.pathname.startsWith(`${PROXY_MOUNT}/`), async route => {
const url = new URL(route.request().url());
url.pathname = url.pathname.slice(PROXY_MOUNT.length);
const response = await route.fetch({ url: url.href });
await route.fulfill({ response });
// A request still in flight when the test ends (a thumbnail the last route started) meets a page that is
// closing: its fetched response is disposed under it. Only that teardown error is ignored.
try {
const response = await route.fetch({ url: url.href });
await route.fulfill({ response });
} catch (error) {
if (!/has been disposed|has been closed|Target page, context or browser has been closed/.test(String(error?.message))) throw error;
}
});
return escaped;
}
tests/js/workspace-activity-store.test.js
+43
View File
@@ -281,3 +281,46 @@ test('summaries are validated and the copy follows the spec', async () => {
assert.equal(outcomeLine(other), '');
assert.deepEqual([-1, 0, 1, 4, 5, 6, 1e9, 0.5].map(backoffDelay), [1000, 1000, 2000, 16000, 30000, 30000, 30000, 1000]);
});
test('pagehide stops polling, closes streams and aborts the in-flight read; pageshow resumes', async () => {
const { createActivityStore } = await source;
const { world, api, ...deps } = harness();
const lifecycle = new EventTarget();
const signals = [];
let answer = null;
const pendingApi = { ...api, get: (url, options) => {
signals.push(options.signal);
world.gets += 1;
if (world.gets === 1) return Promise.resolve({ success: true, jobs: [summary()] });
return new Promise((resolve, reject) => {
answer = resolve;
options.signal.addEventListener('abort', () => reject(Object.assign(new Error('aborted'), { name: 'AbortError' })));
});
} };
const store = createActivityStore({ ...deps, api: pendingApi, pollMs: 15000, lifecycle });
await store.load();
assert.equal(world.open().length, 1);
const poll = world.timers.find(timer => !timer.cleared && timer.delay === 15000);
assert.ok(poll, 'a visible tab polls');
// A read in flight when the page goes away (a closed tab, a test teardown) is aborted, nothing is scheduled again.
const read = store.load();
lifecycle.dispatchEvent(new Event('pagehide'));
await read;
assert.equal(signals.at(-1).aborted, true);
assert.equal(world.open().length, 0);
assert.ok(world.timers.every(timer => timer.cleared || timer.fired));
assert.equal(store.find(ID_A).state, 'running', 'a hidden page marks nothing lost');
await store.load();
assert.equal(world.gets, 2, 'a suspended store reads nothing');
// Back from the cache, it reads the list again and follows the job.
const resumed = Object.assign(new Event('pageshow'), { persisted: true });
lifecycle.dispatchEvent(resumed);
await new Promise(resolve => setImmediate(resolve));
assert.equal(world.gets, 3);
answer({ success: true, jobs: [summary()] });
await new Promise(resolve => setImmediate(resolve));
assert.equal(world.open().length, 1);
world.dispose();
assert.equal(signals.at(-1).aborted, true, 'disposing aborts the store reads');
assert.equal(world.open().length, 0);
});
web/js/workspace/activity-store.mjs
+39 -11
View File
@@ -101,10 +101,15 @@ function defaultConnect(url) {
* @param {{api: {get: Function, post: Function}, scope: {signal: AbortSignal, cleanup: Function},
* connect?: (url: string) => {onmessage: Function|null, onerror: Function|null, close: Function},
* notify?: (message: string) => void, timers?: {setTimeout: Function, clearTimeout: Function},
* pollMs?: number, isVisible?: () => boolean}} options
* pollMs?: number, isVisible?: () => boolean, lifecycle?: EventTarget}} options
*
* `lifecycle` (the window) ends the store's work on `pagehide`: the periodic and backoff reads stop, the streams close
* and a read in flight is aborted, so nothing answers into a page that is going away (a closed tab, a test teardown).
* A `pageshow` from the back-forward cache reads the list again.
*/
export function createActivityStore({ api, scope, connect = defaultConnect, notify = showWorkspaceToast,
timers = globalThis, pollMs = POLL_MS, isVisible = () => globalThis.document?.visibilityState !== 'hidden' } = {}) {
timers = globalThis, pollMs = POLL_MS, isVisible = () => globalThis.document?.visibilityState !== 'hidden',
lifecycle = globalThis.window ?? null } = {}) {
let jobs = [];
let status = 'idle';
let attempt = 0;
@@ -112,6 +117,8 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti
let pollTimer = null;
let reads = 0;
let disposed = false;
let suspended = false;
let reading = null;
const streams = new Map();
const listeners = new Set();
@@ -181,7 +188,7 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti
}
function scheduleResync() {
if (disposed || retryTimer !== null) return;
if (disposed || suspended || retryTimer !== null) return;
const delay = backoffDelay(attempt);
attempt += 1;
retryTimer = timers.setTimeout(() => {
@@ -192,7 +199,7 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti
/** The next periodic read; a hidden tab skips its turn. */
function schedulePoll() {
if (disposed || pollMs <= 0) return;
if (disposed || suspended || pollMs <= 0) return;
if (pollTimer !== null) timers.clearTimeout(pollTimer);
pollTimer = timers.setTimeout(() => {
pollTimer = null;
@@ -203,18 +210,22 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti
/** Read this session's jobs. A failed read marks running jobs ended and tries again after the backoff. */
async function load() {
if (disposed) return;
if (disposed || suspended) return;
const read = ++reads;
// Only the newest read applies, so an older one still in flight is aborted.
reading?.abort();
const controller = new AbortController();
reading = controller;
try {
const result = await api.get(JOBS_URL, { signal: scope.signal, cache: 'no-store' });
// Only the newest read applies; an older one answering late is dropped.
if (disposed || read !== reads) return;
const result = await api.get(JOBS_URL, { signal: controller.signal, cache: 'no-store' });
// An older read answering late is dropped.
if (disposed || suspended || read !== reads) return;
if (!Array.isArray(result?.jobs)) throw new TypeError('Invalid job list');
merge(result.jobs);
status = 'ready';
schedulePoll();
} catch (error) {
if (disposed || scope.signal?.aborted || read !== reads) return;
if (disposed || suspended || scope.signal?.aborted || controller.signal.aborted || read !== reads) return;
markLost();
status = 'error';
scheduleResync();
@@ -254,13 +265,30 @@ export function createActivityStore({ api, scope, connect = defaultConnect, noti
},
};
scope.cleanup(() => {
disposed = true;
/** Stop every timer, stream and read in flight. */
function halt() {
if (retryTimer !== null) timers.clearTimeout(retryTimer);
if (pollTimer !== null) timers.clearTimeout(pollTimer);
retryTimer = null;
pollTimer = null;
reading?.abort();
reading = null;
for (const id of [...streams.keys()]) closeStream(id);
}
const onPageHide = () => { suspended = true; halt(); };
const onPageShow = event => {
if (!suspended || !event.persisted) return;
suspended = false;
load();
};
lifecycle?.addEventListener('pagehide', onPageHide);
lifecycle?.addEventListener('pageshow', onPageShow);
scope.cleanup(() => {
disposed = true;
halt();
lifecycle?.removeEventListener('pagehide', onPageHide);
lifecycle?.removeEventListener('pageshow', onPageShow);
listeners.clear();
});
return store;