fix(browse): serialize navigation — nav-sequence token + per-node load guard
Every async flow that ends by replacing the tree root (refreshListing, rescopeServer, reloadDir, and the app.js back/forward popstate handler) ran without any concurrency guard. Two overlapping listings — a double-click into a folder, a refresh fired mid-load, rapid back/forward — could resolve out of order, so a slow fetch would setRoot/pushState on top of a newer navigation and leave the tree out of sync with state.currentPath and the URL bar. Introduce a shared monotonic nav-sequence token in events.js (beginNav / isCurrentNav, exported so the app.js popstate handler joins the same sequence). Each flow claims a token before its fetch and bails if a newer navigation has started by the time it resolves — last navigation wins, stale ones drop their result before mutating anything. navigateIntoFolder's FS branch is reordered to mutate scope state only after a successful fetch + token check, so a bail leaves the previous scope intact instead of half-swapped. Duplicate-fetch race fixed at the source: tree.loadChildren took only a `loaded` check, so rapid Enter/ArrowRight key-repeat or a double-click landing during a single-click's load fired two concurrent fetches that raced in setChildren. Added a `loading` in-flight flag that serializes per-node loads — the second caller is a no-op until the first resolves. This also removes the need to await the fire-and-forget toggleFolder calls in the keyboard handler. Also surfaces reloadDir fetch failures via statusError instead of swallowing them (the success path's create/rename/delete toast no longer hides a failed refresh). All 6 browse Playwright specs pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
bbbf5326e7
commit
b0d0ff13cd
3 changed files with 66 additions and 11 deletions
|
|
@ -132,8 +132,13 @@
|
||||||
var popQS = new URLSearchParams(location.search);
|
var popQS = new URLSearchParams(location.search);
|
||||||
if (popQS.get('hidden') === '1') window.app.state.showHidden = true;
|
if (popQS.get('hidden') === '1') window.app.state.showHidden = true;
|
||||||
else window.app.state.showHidden = false;
|
else window.app.state.showHidden = false;
|
||||||
|
// Join the shared nav token: rapid back/forward (or back/forward
|
||||||
|
// while an in-tool rescope is mid-flight) must not apply a stale
|
||||||
|
// listing on top of a newer one.
|
||||||
|
var seq = events.beginNav ? events.beginNav() : 0;
|
||||||
try {
|
try {
|
||||||
var es = await loader.fetchServerChildren(path);
|
var es = await loader.fetchServerChildren(path);
|
||||||
|
if (events.isCurrentNav && !events.isCurrentNav(seq)) return;
|
||||||
window.app.state.currentPath = path;
|
window.app.state.currentPath = path;
|
||||||
window.app.state.selectedId = null;
|
window.app.state.selectedId = null;
|
||||||
window.app.state.lastPreviewedNodeId = null;
|
window.app.state.lastPreviewedNodeId = null;
|
||||||
|
|
|
||||||
|
|
@ -133,6 +133,16 @@
|
||||||
} catch (_e) { /* private browsing edge cases */ }
|
} catch (_e) { /* private browsing edge cases */ }
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// Navigation sequence token. Every async flow that ends by replacing
|
||||||
|
// the tree root (refresh, rescope, reload, back/forward popstate)
|
||||||
|
// captures a token before its fetch and bails if a newer navigation
|
||||||
|
// has started by the time it resolves — otherwise a slow listing can
|
||||||
|
// land on top of a newer one and leave the tree out of sync with
|
||||||
|
// state.currentPath / the URL bar.
|
||||||
|
var navSeq = 0;
|
||||||
|
function beginNav() { return ++navSeq; }
|
||||||
|
function isCurrentNav(seq) { return seq === navSeq; }
|
||||||
|
|
||||||
async function refreshListing() {
|
async function refreshListing() {
|
||||||
// Snapshot expanded paths + selection BEFORE setRoot clears the
|
// Snapshot expanded paths + selection BEFORE setRoot clears the
|
||||||
// tree, then re-apply after the new root is in place. Keeps
|
// tree, then re-apply after the new root is in place. Keeps
|
||||||
|
|
@ -141,6 +151,7 @@
|
||||||
// a refresh — including the auto-refresh triggered by the
|
// a refresh — including the auto-refresh triggered by the
|
||||||
// "Show hidden files" toggle.
|
// "Show hidden files" toggle.
|
||||||
var snap = tree.snapshotState();
|
var snap = tree.snapshotState();
|
||||||
|
var seq = beginNav();
|
||||||
if (state.source === 'server') {
|
if (state.source === 'server') {
|
||||||
var raw;
|
var raw;
|
||||||
try {
|
try {
|
||||||
|
|
@ -149,8 +160,10 @@
|
||||||
statusError('Refresh failed: ' + e.message);
|
statusError('Refresh failed: ' + e.message);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
tree.setRoot(raw);
|
tree.setRoot(raw);
|
||||||
await tree.restoreState(snap);
|
await tree.restoreState(snap);
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
tree.render();
|
tree.render();
|
||||||
statusInfo('Refreshed (' + raw.length + ' item'
|
statusInfo('Refreshed (' + raw.length + ' item'
|
||||||
+ (raw.length === 1 ? '' : 's') + ')');
|
+ (raw.length === 1 ? '' : 's') + ')');
|
||||||
|
|
@ -162,8 +175,10 @@
|
||||||
statusError('Refresh failed: ' + e.message);
|
statusError('Refresh failed: ' + e.message);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
tree.setRoot(raw2);
|
tree.setRoot(raw2);
|
||||||
await tree.restoreState(snap);
|
await tree.restoreState(snap);
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
tree.render();
|
tree.render();
|
||||||
statusInfo('Refreshed');
|
statusInfo('Refreshed');
|
||||||
}
|
}
|
||||||
|
|
@ -868,14 +883,20 @@
|
||||||
var loader = window.app.modules.loader;
|
var loader = window.app.modules.loader;
|
||||||
if (!loader) return;
|
if (!loader) return;
|
||||||
if (!dirPath.endsWith('/')) dirPath += '/';
|
if (!dirPath.endsWith('/')) dirPath += '/';
|
||||||
|
var seq = beginNav();
|
||||||
// Root-scope reload — refresh the visible top-level listing.
|
// Root-scope reload — refresh the visible top-level listing.
|
||||||
if (dirPath === state.currentPath) {
|
if (dirPath === state.currentPath) {
|
||||||
|
var es;
|
||||||
try {
|
try {
|
||||||
var es = state.source === 'server'
|
es = state.source === 'server'
|
||||||
? await loader.fetchServerChildren(dirPath)
|
? await loader.fetchServerChildren(dirPath)
|
||||||
: (state.rootHandle ? await loader.fetchFsChildren(state.rootHandle) : []);
|
: (state.rootHandle ? await loader.fetchFsChildren(state.rootHandle) : []);
|
||||||
|
} catch (e) {
|
||||||
|
statusError('Reload failed: ' + (e.message || e));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
tree.setRoot(es);
|
tree.setRoot(es);
|
||||||
} catch (_e) { /* swallow */ }
|
|
||||||
tree.render();
|
tree.render();
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
@ -887,13 +908,18 @@
|
||||||
if (tree.pathFor(n).replace(/\/$/, '') === noSlash) hit = n;
|
if (tree.pathFor(n).replace(/\/$/, '') === noSlash) hit = n;
|
||||||
});
|
});
|
||||||
if (hit) {
|
if (hit) {
|
||||||
|
var raw;
|
||||||
try {
|
try {
|
||||||
var raw = state.source === 'server'
|
raw = state.source === 'server'
|
||||||
? await loader.fetchServerChildren(dirPath)
|
? await loader.fetchServerChildren(dirPath)
|
||||||
: (hit.handle ? await loader.fetchFsChildren(hit.handle) : []);
|
: (hit.handle ? await loader.fetchFsChildren(hit.handle) : []);
|
||||||
|
} catch (e) {
|
||||||
|
statusError('Reload failed: ' + (e.message || e));
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
tree.setChildren(hit.id, raw);
|
tree.setChildren(hit.id, raw);
|
||||||
hit.expanded = true;
|
hit.expanded = true;
|
||||||
} catch (_e) { /* swallow */ }
|
|
||||||
tree.render();
|
tree.render();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
@ -1370,8 +1396,7 @@
|
||||||
}
|
}
|
||||||
if (state.source === 'fs') {
|
if (state.source === 'fs') {
|
||||||
if (!node.handle || node.handle.kind !== 'directory') return;
|
if (!node.handle || node.handle.kind !== 'directory') return;
|
||||||
state.rootHandle = node.handle;
|
var seq = beginNav();
|
||||||
state.currentPath = node.handle.name + '/';
|
|
||||||
var raw;
|
var raw;
|
||||||
try {
|
try {
|
||||||
raw = await loader.fetchFsChildren(node.handle);
|
raw = await loader.fetchFsChildren(node.handle);
|
||||||
|
|
@ -1379,6 +1404,12 @@
|
||||||
statusError('Failed to enter ' + node.name + ': ' + e.message);
|
statusError('Failed to enter ' + node.name + ': ' + e.message);
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
// Mutate scope state only after the fetch succeeds and only if
|
||||||
|
// we're still the latest navigation — a bail here leaves the
|
||||||
|
// previous scope intact rather than half-swapped.
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
|
state.rootHandle = node.handle;
|
||||||
|
state.currentPath = node.handle.name + '/';
|
||||||
tree.setRoot(raw);
|
tree.setRoot(raw);
|
||||||
tree.render();
|
tree.render();
|
||||||
statusInfo('Entered ' + node.name);
|
statusInfo('Entered ' + node.name);
|
||||||
|
|
@ -1389,6 +1420,7 @@
|
||||||
// history.pushState, fetches the new directory listing, and
|
// history.pushState, fetches the new directory listing, and
|
||||||
// re-renders the tree from scratch. Page DOES NOT reload.
|
// re-renders the tree from scratch. Page DOES NOT reload.
|
||||||
async function rescopeServer(url, displayName) {
|
async function rescopeServer(url, displayName) {
|
||||||
|
var seq = beginNav();
|
||||||
var entries;
|
var entries;
|
||||||
try {
|
try {
|
||||||
entries = await loader.fetchServerChildren(url);
|
entries = await loader.fetchServerChildren(url);
|
||||||
|
|
@ -1396,6 +1428,10 @@
|
||||||
statusError('Failed to enter ' + displayName + ': ' + (e.message || e));
|
statusError('Failed to enter ' + displayName + ': ' + (e.message || e));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
// A newer navigation (another dblclick, a refresh, back/forward)
|
||||||
|
// started while this listing was in flight — drop this result so we
|
||||||
|
// don't pushState/setRoot on top of it.
|
||||||
|
if (!isCurrentNav(seq)) return;
|
||||||
state.currentPath = url;
|
state.currentPath = url;
|
||||||
// Selection / preview belong to the old scope; clear them so
|
// Selection / preview belong to the old scope; clear them so
|
||||||
// the new root doesn't carry stale highlight state.
|
// the new root doesn't carry stale highlight state.
|
||||||
|
|
@ -1448,6 +1484,11 @@
|
||||||
// selection). Workflow modules call this after a move/accept so the
|
// selection). Workflow modules call this after a move/accept so the
|
||||||
// tree reflects the change without a manual reload. upload.js already
|
// tree reflects the change without a manual reload. upload.js already
|
||||||
// depends on it being present.
|
// depends on it being present.
|
||||||
refreshListing: refreshListing
|
refreshListing: refreshListing,
|
||||||
|
// Shared navigation-sequence token so the popstate handler (app.js)
|
||||||
|
// can't race the in-tool navigations. beginNav() claims the latest
|
||||||
|
// token; isCurrentNav(seq) reports whether it's still latest.
|
||||||
|
beginNav: beginNav,
|
||||||
|
isCurrentNav: isCurrentNav
|
||||||
};
|
};
|
||||||
})();
|
})();
|
||||||
|
|
|
||||||
|
|
@ -512,7 +512,14 @@
|
||||||
// it as a directory handle; members
|
// it as a directory handle; members
|
||||||
// become ordinary dir/file nodes
|
// become ordinary dir/file nodes
|
||||||
async function loadChildren(node) {
|
async function loadChildren(node) {
|
||||||
if (node.loaded) return;
|
if (node.loaded || node.loading) return;
|
||||||
|
// In-flight guard: a folder can be (re)toggled while its first
|
||||||
|
// load is still pending — rapid Enter/ArrowRight key-repeat, or a
|
||||||
|
// double-click landing during a single-click's load. Without this,
|
||||||
|
// both calls pass the !loaded check and fire duplicate fetches that
|
||||||
|
// race in setChildren. The flag serializes per-node so the second
|
||||||
|
// caller is a no-op until the first resolves.
|
||||||
|
node.loading = true;
|
||||||
try {
|
try {
|
||||||
if (node.isZip && state.source === 'server' && !zipNestedInsideZip(node)) {
|
if (node.isZip && state.source === 'server' && !zipNestedInsideZip(node)) {
|
||||||
setChildren(node.id, await loader.fetchServerChildren(pathFor(node) + '/'));
|
setChildren(node.id, await loader.fetchServerChildren(pathFor(node) + '/'));
|
||||||
|
|
@ -532,6 +539,8 @@
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
window.app.modules.events.statusError(
|
window.app.modules.events.statusError(
|
||||||
'Failed to load ' + node.name + ': ' + e.message);
|
'Failed to load ' + node.name + ': ' + e.message);
|
||||||
|
} finally {
|
||||||
|
node.loading = false;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue