From 6f6c688058e73851114dc1c75e89e20d8704e6cf Mon Sep 17 00:00:00 2001 From: Eoghan Murray Date: Wed, 1 Apr 2026 12:00:00 +0800 Subject: [PATCH] Add more concrete "node not found" warnings (#620) * Show whether a node was not found because it was never there, or because some other mutation already removed it * Be more careful applying mutations as replay-stopping things can go wrong, e.g. node removal parent belongs to a different FullSnapshot and has e.g. a text nodeType * Don't warn when a modified node has been removed in the same mutation - or the parent node for a removed node has been already removed --- src/replay/index.ts | 46 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 39 insertions(+), 7 deletions(-) diff --git a/src/replay/index.ts b/src/replay/index.ts index 4ba93330..f8c91582 100644 --- a/src/replay/index.ts +++ b/src/replay/index.ts @@ -769,7 +769,11 @@ export class Replayer { d.attributes.forEach((m) => this.treeIndex.attribute(m)); d.removes.forEach((m) => this.treeIndex.remove(m, this.mirror)); } - this.applyMutation(d, isSync); + try { + this.applyMutation(d, isSync); + } catch (error) { + this.warn(`Exception in mutation ${error.message || error}`, d); + } break; } case IncrementalSource.Drag: @@ -1056,6 +1060,10 @@ export class Replayer { d.removes.forEach((mutation) => { const target = this.mirror.getNode(mutation.id); if (!target) { + if (d.removes.find((r) => r.id === mutation.parentId)) { + // no need to warn, parent was already removed + return; + } return this.warnNodeNotFound(d, mutation.id); } let parent: INode | null | ShadowRoot = this.mirror.getNode( @@ -1070,20 +1078,28 @@ export class Replayer { // target may be removed with its parents before this.mirror.removeNodeFromMap(target); if (parent) { + let realTarget = null; const realParent = '__sn' in parent ? this.fragmentParentMap.get(parent) : undefined; if (realParent && realParent.contains(target)) { - realParent.removeChild(target); + parent = realParent; } else if (this.fragmentParentMap.has(target)) { /** * the target itself is a fragment document and it's not in the dom * so we should remove the real target from its parent */ - const realTarget = this.fragmentParentMap.get(target)!; - parent.removeChild(realTarget); + realTarget = this.fragmentParentMap.get(target)!; this.fragmentParentMap.delete(target); - } else { + target = realTarget; + } + try { parent.removeChild(target); + } catch (error) { + if (error instanceof DOMException) { + this.warn('parent could not remove child in mutation', parent, realParent, target, realTarget , d); + } else { + throw error; + } } } }); @@ -1290,6 +1306,10 @@ export class Replayer { d.texts.forEach((mutation) => { let target = this.mirror.getNode(mutation.id); if (!target) { + if (d.removes.find((r) => r.id === mutation.id)) { + // no need to warn, element was already removed + return; + } return this.warnNodeNotFound(d, mutation.id); } /** @@ -1303,6 +1323,10 @@ export class Replayer { d.attributes.forEach((mutation) => { let target = this.mirror.getNode(mutation.id); if (!target) { + if (d.removes.find((r) => r.id === mutation.id)) { + // no need to warn, element was already removed + return; + } return this.warnNodeNotFound(d, mutation.id); } if (this.fragmentParentMap.has(target)) { @@ -1584,7 +1608,11 @@ export class Replayer { } private warnNodeNotFound(d: incrementalData, id: number) { - this.warn(`Node with id '${id}' not found in`, d); + if (this.treeIndex.removeIdSet.has(id)) { + this.warn(`Node with id '${id}' was previously removed. `, d); + } else { + this.warn(`Node with id '${id}' not found. `, d); + } } private warnCanvasMutationFailed( @@ -1602,7 +1630,11 @@ export class Replayer { * is microtask, so events fired on a removed DOM may emit * snapshots in the reverse order. */ - this.debug(REPLAY_CONSOLE_PREFIX, `Node with id '${id}' not found in`, d); + if (this.treeIndex.removeIdSet.has(id)) { + this.debug(REPLAY_CONSOLE_PREFIX, `Node with id '${id}' was previously removed. `, d); + } else { + this.debug(REPLAY_CONSOLE_PREFIX, `Node with id '${id}' not found. `, d); + } } private warn(...args: Parameters) {