From a6e044fbb8d5ba5171899823cc4c31073fa4a0c3 Mon Sep 17 00:00:00 2001 From: vrazuvaev Date: Fri, 10 Jul 2026 18:22:21 +0200 Subject: [PATCH 1/2] Report full data path in incompatible-update invariants When an incompatible-update invariant fires on a non-addressable value (a CompositeNull/CompositeUndefined, whose source data is null/undefined), getDataPathForDebugging cannot locate it, so the message fell back to only the last field/index segment (e.g. 'value' instead of 'listContainer.items.0.value'). Thread the failing value's parent chunk (always an addressable ObjectChunk/CompositeListChunk) into the location and derive the path from the parent plus the field/index step, so the full path is reported even for non-addressable values. The enclosing (in ) node is likewise resolved from the parent when needed. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...-8f2a1c4d-6b3e-4a1f-9c7d-2e5b8a0d1f34.json | 7 ++ .../src/forest/__tests__/updateObject.test.ts | 52 ++++++++++++++ .../src/forest/updateObject.ts | 70 ++++++++++++++----- 3 files changed, 111 insertions(+), 18 deletions(-) create mode 100644 change/@graphitation-apollo-forest-run-8f2a1c4d-6b3e-4a1f-9c7d-2e5b8a0d1f34.json diff --git a/change/@graphitation-apollo-forest-run-8f2a1c4d-6b3e-4a1f-9c7d-2e5b8a0d1f34.json b/change/@graphitation-apollo-forest-run-8f2a1c4d-6b3e-4a1f-9c7d-2e5b8a0d1f34.json new file mode 100644 index 000000000..22601413a --- /dev/null +++ b/change/@graphitation-apollo-forest-run-8f2a1c4d-6b3e-4a1f-9c7d-2e5b8a0d1f34.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Report the full data path (not just the last field/index) in incompatible-update invariant messages when the failing value is a non-addressable CompositeNull/CompositeUndefined", + "packageName": "@graphitation/apollo-forest-run", + "email": "vrazuvaev@microsoft.com_msteamsmdb", + "dependentChangeType": "patch" +} diff --git a/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts b/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts index 5f8d28170..486a7d408 100644 --- a/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts +++ b/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts @@ -83,6 +83,10 @@ const NESTED_INCOMPATIBLE_OBJECT_DIFF_ERROR = 'Invariant violation: Failed to update "query BaseFeedQuery" ' + "at path listContainer.items.0.value (in Entity): expected CompositeList, got ObjectDifference"; +const NESTED_INCOMPATIBLE_NULL_DIFF_ERROR = + 'Invariant violation: Failed to update "query BaseFeedQuery" ' + + "at path listContainer.items.0.value (in Entity): expected CompositeNull, got ObjectDifference"; + describe("incompatible object difference invariant", () => { it("reports operation and path for a field two levels deep below a list", () => { const cache = new ForestRun(); @@ -131,4 +135,52 @@ describe("incompatible object difference invariant", () => { NESTED_INCOMPATIBLE_OBJECT_DIFF_ERROR, ); }); + + it("reports the full path when the failing value is a nested CompositeNull", () => { + const cache = new ForestRun(); + + // Same setup as above, but here the same Entity's "value" is a null (with a + // sub-selection, i.e. a CompositeNull) under "listContainer" and an object under + // "objectContainer". A CompositeNull's source data is `null`, so it is not + // addressable by the path utils on its own - the reported path must therefore be + // derived from its parent chunk, yielding the full "listContainer.items.0.value" + // rather than just the final "value" segment. + cache.write({ + query: nestedFeedListQuery, + result: { + objectContainer: { + __typename: "Container", + id: "object", + items: [{ __typename: "Entity", id: "1", value: { note: "old" } }], + }, + listContainer: { + __typename: "Container", + id: "list", + items: [{ __typename: "Entity", id: "1", value: null }], + }, + }, + }); + + const run = () => + cache.write({ + query: nestedFeedObjectQuery, + result: { + objectContainer: { + __typename: "Container", + id: "object", + items: [{ __typename: "Entity", id: "1", value: { note: "new" } }], + }, + }, + }); + + let error: unknown; + try { + run(); + } catch (thrown) { + error = thrown; + } + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message).toBe(NESTED_INCOMPATIBLE_NULL_DIFF_ERROR); + }); }); diff --git a/packages/apollo-forest-run/src/forest/updateObject.ts b/packages/apollo-forest-run/src/forest/updateObject.ts index 0bf8841cb..98528312d 100644 --- a/packages/apollo-forest-run/src/forest/updateObject.ts +++ b/packages/apollo-forest-run/src/forest/updateObject.ts @@ -101,7 +101,8 @@ function updateObjectValue( const updated = updateValue(context, value, fieldDiff, { kind: "field", - fieldName: fieldInfo.name, + parent: base, + field: fieldInfo, }); if (valueIsMissing && updated !== undefined) { @@ -152,8 +153,8 @@ function updateObjectValue( } type UpdateValueLocation = - | { kind: "field"; fieldName: string } - | { kind: "listItem"; index: number }; + | { kind: "field"; parent: GraphChunk; field: FieldInfo } + | { kind: "listItem"; parent: GraphChunk; index: number }; function updateValue( context: UpdateTreeContext, @@ -296,46 +297,79 @@ function updateFailureMessage( location?: UpdateValueLocation, ): string { const prefix = `Failed to update "${context.operation.debugName}"`; - const basePath = chunkPath(context, base); - const pathString = basePath?.length - ? basePath.join(".") - : location - ? describeLocation(location) - : undefined; - const nodeClause = nodeTypeClause(context, base); + const pathString = resolveFailurePath(context, base, location); + const nodeClause = nodeTypeClause(context, base, location); return pathString ? `${prefix} at path ${pathString}${nodeClause}: ${detail}` : `${prefix}${nodeClause}: ${detail}`; } +// Full path to the failing value, e.g. "listContainer.items.0.value". +// The failing value itself may not be addressable by the path utils (e.g. a CompositeNull +// or CompositeUndefined, whose source data is null/undefined and cannot be located in the +// tree). In that case its own path resolves to nothing and we would otherwise only report +// the final field/index. So we prefer to derive the path from the failing value's parent +// chunk (always an ObjectChunk or CompositeListChunk, which *is* addressable) and append +// the field/index step, falling back to the value's own path or the bare step. +function resolveFailurePath( + context: UpdateTreeContext, + base: GraphChunk, + location?: UpdateValueLocation, +): string | undefined { + if (location) { + const parentPath = chunkPath(context, location.parent); + if (parentPath) { + return parentPath.concat(describeLocation(location)).join("."); + } + } + const basePath = chunkPath(context, base); + if (basePath?.length) { + return basePath.join("."); + } + return location ? describeLocation(location) : undefined; +} + function describeLocation(location: UpdateValueLocation): string { return location.kind === "field" - ? location.fieldName + ? location.field.dataKey : String(location.index); } // Best-effort " (in )" clause naming the closest enclosing node's __typename. // Defensive: the tree may already be inconsistent when an invariant fires, so it is // wrapped in try/catch and falls back to an empty clause (and aggregates are skipped, -// as they are not addressable by the path utils). +// as they are not addressable by the path utils). When the failing value itself is not +// addressable (e.g. a CompositeNull), the enclosing node is resolved from its parent. function nodeTypeClause( env: { findParent: ParentLocator }, chunk: GraphChunk, + location?: UpdateValueLocation, ): string { - if ( - (!Value.isObjectValue(chunk) && !Value.isCompositeListValue(chunk)) || - chunk.isAggregate - ) { + const anchor = nodeAnchor(chunk) ?? nodeAnchor(location?.parent); + if (!anchor) { return ""; } try { - const node = Value.findClosestNode(chunk, env.findParent); + const node = Value.findClosestNode(anchor, env.findParent); return node.type ? ` (in ${node.type})` : ""; } catch { return ""; } } +function nodeAnchor( + chunk: GraphChunk | undefined, +): ObjectChunk | CompositeListChunk | undefined { + if ( + chunk && + (Value.isObjectValue(chunk) || Value.isCompositeListValue(chunk)) && + !chunk.isAggregate + ) { + return chunk; + } + return undefined; +} + // Path of a chunk within its tree, e.g. ["listContainer", "items", 0, "value"]. // Returns undefined for values that are not addressable by the path utils (e.g. leaf values). // Wrapped in try/catch: this runs while reporting an invariant, so the tree may already @@ -406,7 +440,7 @@ function updateCompositeListValue( context, Value.resolveListItemChunk(base, index), itemDiff, - { kind: "listItem", index }, + { kind: "listItem", parent: base, index }, ); if (updatedValue === base.data[index]) { continue; From a153ade5f3d65e17bc44fef8c0094025e69b365e Mon Sep 17 00:00:00 2001 From: vrazuvaev Date: Fri, 10 Jul 2026 18:36:46 +0200 Subject: [PATCH 2/2] Harden invariant message builder against exceptions during path resolution resolveFailurePath now derives the path from a nested property (location.field.dataKey) rather than an eagerly-captured primitive, so a malformed field could throw while building the invariant message and mask the real invariant. Wrap the whole derivation in try/catch, consistent with the other best-effort diagnostic helpers (chunkPath, nodeTypeClause), so path resolution can never throw over the invariant it is reporting. Add a regression test that forces findParent to throw during both path and enclosing-node resolution and asserts the invariant message still surfaces. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .../src/forest/__tests__/updateObject.test.ts | 35 +++++++++++++++++++ .../src/forest/updateObject.ts | 24 ++++++++----- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts b/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts index 486a7d408..3f709e19f 100644 --- a/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts +++ b/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts @@ -35,6 +35,41 @@ describe("replaceValue invariant", () => { 'Invariant violation: Failed to update "query ReplaceScalar": cannot replace Scalar with Object (of type Widget)', ); }); + + it("still reports the invariant when path/node resolution throws", () => { + const operation = createTestOperation(gql` + query ReplaceObject { + node { + id + } + } + `); + // base is an addressable, keyless embedded object, so both path resolution and + // enclosing-node resolution must ascend the tree via findParent. A corrupt tree can + // make findParent throw while an invariant is being reported; the message builder must + // swallow that and still surface the invariant rather than the internal exception. + const objectBase = resolveFieldValue( + generateChunk(`query ObjectSource { embedded { field } }`).value, + "embedded", + ) as ObjectChunk; + const listReplacement = resolveFieldValue( + generateChunk(`query ListSource { items @mock(count: 2) { id } }`).value, + "items", + ); + + const context = { + operation, + findParent: () => { + throw new Error("corrupt tree: findParent exploded"); + }, + } as unknown as UpdateTreeContext; + + expect(() => + replaceValue(context, objectBase as any, listReplacement as any), + ).toThrow( + 'Invariant violation: Failed to update "query ReplaceObject": cannot replace Object with CompositeList', + ); + }); }); const nestedFeedListQuery = gql` diff --git a/packages/apollo-forest-run/src/forest/updateObject.ts b/packages/apollo-forest-run/src/forest/updateObject.ts index 98528312d..13ff864e3 100644 --- a/packages/apollo-forest-run/src/forest/updateObject.ts +++ b/packages/apollo-forest-run/src/forest/updateObject.ts @@ -316,17 +316,23 @@ function resolveFailurePath( base: GraphChunk, location?: UpdateValueLocation, ): string | undefined { - if (location) { - const parentPath = chunkPath(context, location.parent); - if (parentPath) { - return parentPath.concat(describeLocation(location)).join("."); + try { + if (location) { + const parentPath = chunkPath(context, location.parent); + if (parentPath) { + return parentPath.concat(describeLocation(location)).join("."); + } } + const basePath = chunkPath(context, base); + if (basePath?.length) { + return basePath.join("."); + } + return location ? describeLocation(location) : undefined; + } catch { + // Diagnostics only: the tree is already inconsistent when an invariant fires, + // so path resolution is best-effort and must never throw over the invariant. + return undefined; } - const basePath = chunkPath(context, base); - if (basePath?.length) { - return basePath.join("."); - } - return location ? describeLocation(location) : undefined; } function describeLocation(location: UpdateValueLocation): string {