From 1f23d9c6d66e6e57e70451a73ad7a8c3ba3a190d Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Sat, 20 Jun 2026 17:17:47 +0200 Subject: [PATCH 1/3] Report incompatible object diff invariants --- .../src/__tests__/regression.test.ts | 189 ++++++++++++++++++ .../src/forest/updateObject.ts | 101 +++++++++- 2 files changed, 287 insertions(+), 3 deletions(-) diff --git a/packages/apollo-forest-run/src/__tests__/regression.test.ts b/packages/apollo-forest-run/src/__tests__/regression.test.ts index fdbb4e591..236a01148 100644 --- a/packages/apollo-forest-run/src/__tests__/regression.test.ts +++ b/packages/apollo-forest-run/src/__tests__/regression.test.ts @@ -1,3 +1,4 @@ +// import { InMemoryCache } from "@apollo/client/cache"; import { gql } from "../__tests__/helpers/descriptor"; import { ForestRun } from "../ForestRun"; @@ -1086,6 +1087,194 @@ test("does not crash when object diff is applied to list field with key collisio expect(run).not.toThrow(); }); +const inconsistentEntityQuery = gql` + { + objectVariant { + __typename + id + value { + note + } + } + listVariant { + __typename + id + value { + note + } + } + } +`; +const objectVariantQuery = gql` + { + objectVariant { + __typename + id + value { + note + } + } + } +`; + +const INCOMPATIBLE_OBJECT_DIFF_ERROR = + 'Invariant violation: cannot apply ObjectDifference to CompositeList at field "value" while updating a query operation; ' + + "expected value kind=Object, actual value kind=CompositeList; " + + "difference metadata: dirty fields=1, pending fields=1, field states=1. " + + "This indicates inconsistent structural representations for the same normalized object. " + + "Likely causes include inconsistent response shapes for a repeated normalized entity or a cache-key collision. " + + "Object keys, operation names, aliases, arguments, variables, type names, and cached values are omitted to protect privacy."; + +function expectInvariantViolation(run: () => unknown, message: string) { + let error: unknown; + try { + run(); + } catch (thrown) { + error = thrown; + } + + expect(error).toBeInstanceOf(Error); + expect((error as Error).message.startsWith("Invariant violation")).toBe(true); + expect((error as Error).message).toBe(message); +} + +test("reports an object diff reaching an inconsistent repeated entity", () => { + const cache = new ForestRun(); + + cache.write({ + query: inconsistentEntityQuery, + result: { + objectVariant: { + __typename: "Entity", + id: "1", + value: { note: "old" }, + }, + listVariant: { + __typename: "Entity", + id: "1", + value: [{ note: "old" }], + }, + }, + }); + + const run = () => + cache.write({ + query: objectVariantQuery, + result: { + objectVariant: { + __typename: "Entity", + id: "1", + value: { note: "new" }, + }, + }, + }); + + expectInvariantViolation(run, INCOMPATIBLE_OBJECT_DIFF_ERROR); +}); + +test("reports an object diff reaching a cache key collision", () => { + const cache = new ForestRun({ + dataIdFromObject: (object: any) => object.id, + }); + + cache.write({ + query: inconsistentEntityQuery, + result: { + objectVariant: { + __typename: "ObjectEntity", + id: "1", + value: { note: "old" }, + }, + listVariant: { + __typename: "ListEntity", + id: "1", + value: [{ note: "old" }], + }, + }, + }); + + const run = () => + cache.write({ + query: objectVariantQuery, + result: { + objectVariant: { + __typename: "ObjectEntity", + id: "1", + value: { note: "new" }, + }, + }, + }); + + expectInvariantViolation(run, INCOMPATIBLE_OBJECT_DIFF_ERROR); +}); + +// Apollo currently replaces these incompatible values. Keep this comparison +// available in case we decide to implement Apollo-compatible recovery later. +test.skip.each([ + { + name: "an inconsistent repeated entity", + config: undefined, + objectTypename: "Entity", + listTypename: "Entity", + entityKey: "Entity:1", + }, + { + name: "a cache key collision", + config: { dataIdFromObject: (object: any) => object.id }, + objectTypename: "ObjectEntity", + listTypename: "ListEntity", + entityKey: "1", + }, +])( + "stock Apollo replaces incompatible values for $name", + async ({ config, objectTypename, listTypename, entityKey }) => { + const { InMemoryCache } = await import("@apollo/client/cache"); + const cache = new InMemoryCache(config); + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + + try { + cache.writeQuery({ + query: inconsistentEntityQuery, + data: { + objectVariant: { + __typename: objectTypename, + id: "1", + value: { note: "old" }, + }, + listVariant: { + __typename: listTypename, + id: "1", + value: [{ note: "old" }], + }, + }, + }); + cache.writeQuery({ + query: objectVariantQuery, + data: { + objectVariant: { + __typename: objectTypename, + id: "1", + value: { note: "new" }, + }, + }, + }); + + const expectedEntity = { + __typename: objectTypename, + id: "1", + value: { note: "new" }, + }; + expect(cache.extract()[entityKey]).toEqual(expectedEntity); + expect(cache.readQuery({ query: inconsistentEntityQuery })).toEqual({ + objectVariant: expectedEntity, + listVariant: expectedEntity, + }); + } finally { + warn.mockRestore(); + } + }, +); + test("matches apollo InMemoryCache behavior on incorrect cache overwrites", () => { const listQuery = gql` query ListQuery { diff --git a/packages/apollo-forest-run/src/forest/updateObject.ts b/packages/apollo-forest-run/src/forest/updateObject.ts index 706ac5ced..24bad6564 100644 --- a/packages/apollo-forest-run/src/forest/updateObject.ts +++ b/packages/apollo-forest-run/src/forest/updateObject.ts @@ -98,7 +98,10 @@ function updateObjectValue( continue; } - const updated = updateValue(context, value, fieldDiff); + const updated = updateValue(context, value, fieldDiff, { + kind: "field", + fieldName: fieldInfo.name, + }); if (valueIsMissing && updated !== undefined) { context.missingFields.get(base.data)?.delete(fieldInfo); @@ -147,22 +150,45 @@ function updateObjectValue( return copy ?? base.data; } +type UpdateValueLocation = + | { kind: "field"; fieldName: string } + | { kind: "listItem"; index: number }; + function updateValue( context: UpdateTreeContext, base: GraphChunk, difference: ValueDifference, + location: UpdateValueLocation, ): SourceValue | undefined { switch (difference.kind) { case DifferenceKind.Replacement: return replaceValue(context, base, difference.newValue); case DifferenceKind.ObjectDifference: { - assert(Value.isObjectValue(base)); + assert( + Value.isObjectValue(base), + incompatibleDifferenceMessage( + context, + base, + difference, + location, + "Object", + ), + ); return updateObjectValue(context, base, difference); } case DifferenceKind.CompositeListDifference: { - assert(Value.isCompositeListValue(base)); + assert( + Value.isCompositeListValue(base), + incompatibleDifferenceMessage( + context, + base, + difference, + location, + "CompositeList", + ), + ); return updateCompositeListValue(context, base, difference); } @@ -179,6 +205,74 @@ function updateValue( } } +function incompatibleDifferenceMessage( + context: UpdateTreeContext, + base: GraphChunk, + difference: ObjectDifference | CompositeListDifference, + location: UpdateValueLocation, + expectedKind: "Object" | "CompositeList", +): string { + const locationDescription = + location.kind === "field" + ? `field "${location.fieldName}"` + : `list item at index ${location.index}`; + const differenceKind = Difference.isObjectDifference(difference) + ? "ObjectDifference" + : "CompositeListDifference"; + const differenceState = Difference.isObjectDifference(difference) + ? `dirty fields=${difference.dirtyFields?.size ?? 0}, pending fields=${ + difference.fieldQueue.size + }, field states=${difference.fieldState.size}` + : `dirty items=${difference.dirtyItems?.size ?? 0}, pending items=${ + difference.itemQueue.size + }, item states=${difference.itemState.size}, layout change=${Boolean( + difference.layout, + )}`; + + return ( + `cannot apply ${differenceKind} to ${valueKindName( + base, + )} at ${locationDescription} ` + + `while updating a ${context.operation.definition.operation} operation; ` + + `expected value kind=${expectedKind}, actual value kind=${valueKindName( + base, + )}; ` + + `difference metadata: ${differenceState}. ` + + `This indicates inconsistent structural representations for the same normalized object. ` + + `Likely causes include inconsistent response shapes for a repeated normalized entity or a cache-key collision. ` + + `Object keys, operation names, aliases, arguments, variables, type names, and cached values are omitted to protect privacy.` + ); +} + +function valueKindName(value: GraphChunk): string { + if (Value.isScalarValue(value)) { + return "Scalar"; + } + if (Value.isLeafNull(value)) { + return "LeafNull"; + } + switch (value.kind) { + case ValueKind.Object: + return "Object"; + case ValueKind.CompositeList: + return "CompositeList"; + case ValueKind.CompositeNull: + return "CompositeNull"; + case ValueKind.CompositeUndefined: + return "CompositeUndefined"; + case ValueKind.LeafList: + return "LeafList"; + case ValueKind.LeafError: + return "LeafError"; + case ValueKind.LeafUndefined: + return "LeafUndefined"; + case ValueKind.ComplexScalar: + return "ComplexScalar"; + default: + return assertNever(value); + } +} + function updateCompositeListValue( context: UpdateTreeContext, base: CompositeListChunk, @@ -202,6 +296,7 @@ function updateCompositeListValue( context, Value.resolveListItemChunk(base, index), itemDiff, + { kind: "listItem", index }, ); if (updatedValue === base.data[index]) { continue; From 16951105a362725de3e354ecf0ae2d2ac5781732 Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Sat, 20 Jun 2026 17:24:28 +0200 Subject: [PATCH 2/3] Remove obsolete Apollo cache regression test --- .../src/__tests__/regression.test.ts | 68 ------------------- 1 file changed, 68 deletions(-) diff --git a/packages/apollo-forest-run/src/__tests__/regression.test.ts b/packages/apollo-forest-run/src/__tests__/regression.test.ts index 236a01148..4c8f242b4 100644 --- a/packages/apollo-forest-run/src/__tests__/regression.test.ts +++ b/packages/apollo-forest-run/src/__tests__/regression.test.ts @@ -1,4 +1,3 @@ -// import { InMemoryCache } from "@apollo/client/cache"; import { gql } from "../__tests__/helpers/descriptor"; import { ForestRun } from "../ForestRun"; @@ -1208,73 +1207,6 @@ test("reports an object diff reaching a cache key collision", () => { expectInvariantViolation(run, INCOMPATIBLE_OBJECT_DIFF_ERROR); }); -// Apollo currently replaces these incompatible values. Keep this comparison -// available in case we decide to implement Apollo-compatible recovery later. -test.skip.each([ - { - name: "an inconsistent repeated entity", - config: undefined, - objectTypename: "Entity", - listTypename: "Entity", - entityKey: "Entity:1", - }, - { - name: "a cache key collision", - config: { dataIdFromObject: (object: any) => object.id }, - objectTypename: "ObjectEntity", - listTypename: "ListEntity", - entityKey: "1", - }, -])( - "stock Apollo replaces incompatible values for $name", - async ({ config, objectTypename, listTypename, entityKey }) => { - const { InMemoryCache } = await import("@apollo/client/cache"); - const cache = new InMemoryCache(config); - const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); - - try { - cache.writeQuery({ - query: inconsistentEntityQuery, - data: { - objectVariant: { - __typename: objectTypename, - id: "1", - value: { note: "old" }, - }, - listVariant: { - __typename: listTypename, - id: "1", - value: [{ note: "old" }], - }, - }, - }); - cache.writeQuery({ - query: objectVariantQuery, - data: { - objectVariant: { - __typename: objectTypename, - id: "1", - value: { note: "new" }, - }, - }, - }); - - const expectedEntity = { - __typename: objectTypename, - id: "1", - value: { note: "new" }, - }; - expect(cache.extract()[entityKey]).toEqual(expectedEntity); - expect(cache.readQuery({ query: inconsistentEntityQuery })).toEqual({ - objectVariant: expectedEntity, - listVariant: expectedEntity, - }); - } finally { - warn.mockRestore(); - } - }, -); - test("matches apollo InMemoryCache behavior on incorrect cache overwrites", () => { const listQuery = gql` query ListQuery { From 13eba11480f8ca39c34453702ae5cb3eced80c2e Mon Sep 17 00:00:00 2001 From: Vladimir Razuvaev Date: Fri, 26 Jun 2026 10:37:43 +0200 Subject: [PATCH 3/3] Enrich incompatible update invariant messages with diagnostics (#681) * Enrich incompatible update invariant messages with diagnostics Reformat invariant-violation messages across updateObject, updateTree, traverse, and draftHelpers to follow a consistent broader-to-specific shape: operation debugName, then path, then the enclosing node __typename, then the observed value (including the incoming value's type where it can be recovered). All path and type extraction is wrapped in try/catch since the cache state is already corrupt when an invariant fires, and no node keys are interpolated to avoid exposing entity identifiers. Adds co-located unit tests for every new message with named operations and exact-message assertions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix lint: normalize line endings and formatting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Change files * Defer invariant message construction to the failure path The enriched diagnostic messages walk the data path and look up __typenames, which is expensive. Passing them as the second argument to assert() built the full string on every call, including the success path, causing a 7-10% regression in the write-array benchmarks. Guard each message-bearing assert with an explicit condition check so the message builder only runs when the invariant actually fails, matching the existing if (!cond) { assert(false, ...) } pattern already used in traverse.ts and draftHelpers.ts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: vrazuvaev Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- ...-1d051b51-0431-4bbf-ab8a-ee2476211a19.json | 7 + .../src/__tests__/regression.test.ts | 18 +- .../src/cache/__tests__/draftHelpers.test.ts | 36 +++ .../src/cache/draftHelpers.ts | 71 +++++- .../src/forest/__tests__/updateObject.test.ts | 134 ++++++++++ .../src/forest/__tests__/updateTree.test.ts | 53 ++++ .../src/forest/updateObject.ts | 233 ++++++++++++++---- .../src/forest/updateTree.ts | 36 ++- .../src/values/__tests__/traverse.test.ts | 30 +++ .../apollo-forest-run/src/values/traverse.ts | 36 ++- 10 files changed, 585 insertions(+), 69 deletions(-) create mode 100644 change/@graphitation-apollo-forest-run-1d051b51-0431-4bbf-ab8a-ee2476211a19.json create mode 100644 packages/apollo-forest-run/src/cache/__tests__/draftHelpers.test.ts create mode 100644 packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts diff --git a/change/@graphitation-apollo-forest-run-1d051b51-0431-4bbf-ab8a-ee2476211a19.json b/change/@graphitation-apollo-forest-run-1d051b51-0431-4bbf-ab8a-ee2476211a19.json new file mode 100644 index 000000000..c63272eeb --- /dev/null +++ b/change/@graphitation-apollo-forest-run-1d051b51-0431-4bbf-ab8a-ee2476211a19.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Enrich incompatible update invariant violation messages with operation, path, and type diagnostics", + "packageName": "@graphitation/apollo-forest-run", + "email": "vrazuvaev@microsoft.com_msteamsmdb", + "dependentChangeType": "patch" +} diff --git a/packages/apollo-forest-run/src/__tests__/regression.test.ts b/packages/apollo-forest-run/src/__tests__/regression.test.ts index 4c8f242b4..ebe407336 100644 --- a/packages/apollo-forest-run/src/__tests__/regression.test.ts +++ b/packages/apollo-forest-run/src/__tests__/regression.test.ts @@ -1117,12 +1117,16 @@ const objectVariantQuery = gql` `; const INCOMPATIBLE_OBJECT_DIFF_ERROR = - 'Invariant violation: cannot apply ObjectDifference to CompositeList at field "value" while updating a query operation; ' + - "expected value kind=Object, actual value kind=CompositeList; " + - "difference metadata: dirty fields=1, pending fields=1, field states=1. " + - "This indicates inconsistent structural representations for the same normalized object. " + - "Likely causes include inconsistent response shapes for a repeated normalized entity or a cache-key collision. " + - "Object keys, operation names, aliases, arguments, variables, type names, and cached values are omitted to protect privacy."; + 'Invariant violation: Failed to update "query {\n' + + " objectVariant {...}\n" + + " listVariant {...}\n" + + '}" at path listVariant.value (in Entity): expected CompositeList, got ObjectDifference'; + +const INCOMPATIBLE_OBJECT_DIFF_ERROR_KEY_COLLISION = + 'Invariant violation: Failed to update "query {\n' + + " objectVariant {...}\n" + + " listVariant {...}\n" + + '}" at path listVariant.value (in ListEntity): expected CompositeList, got ObjectDifference'; function expectInvariantViolation(run: () => unknown, message: string) { let error: unknown; @@ -1204,7 +1208,7 @@ test("reports an object diff reaching a cache key collision", () => { }, }); - expectInvariantViolation(run, INCOMPATIBLE_OBJECT_DIFF_ERROR); + expectInvariantViolation(run, INCOMPATIBLE_OBJECT_DIFF_ERROR_KEY_COLLISION); }); test("matches apollo InMemoryCache behavior on incorrect cache overwrites", () => { diff --git a/packages/apollo-forest-run/src/cache/__tests__/draftHelpers.test.ts b/packages/apollo-forest-run/src/cache/__tests__/draftHelpers.test.ts new file mode 100644 index 000000000..58b4ade26 --- /dev/null +++ b/packages/apollo-forest-run/src/cache/__tests__/draftHelpers.test.ts @@ -0,0 +1,36 @@ +import { getEmbeddedObjectChunks } from "../draftHelpers"; +import { generateChunk } from "../../__tests__/helpers/values"; +import { + createParentLocator, + getGraphValueReference, +} from "../../values/traverse"; +import { resolveFieldValue } from "../../values/resolve"; +import { CompositeListChunk, NodeChunk } from "../../values/types"; + +describe("getEmbeddedObjectChunks invariant", () => { + it("reports operation and path when the embedded value is a list", () => { + const query = `query EmbeddedListTest { + node { + __typename @mock(value: "Node") + id + items @mock(count: 2) { + __typename @mock(value: "Item") + value + } + } + }`; + const { value: root, dataMap } = generateChunk(query); + const env = { findParent: createParentLocator(dataMap) }; + + const node = resolveFieldValue(root, "node") as NodeChunk; + const items = resolveFieldValue(node, "items") as CompositeListChunk; + + // The reference resolves to a composite list, but getEmbeddedObjectChunks + // expects an embedded object at that location. + const ref = getGraphValueReference(env, items); + + expect(() => [...getEmbeddedObjectChunks(env, [node], ref)]).toThrow( + 'Invariant violation: Failed to resolve embedded object in "query EmbeddedListTest" at path node.items (in Node): expected an embedded object, got a list of Item', + ); + }); +}); diff --git a/packages/apollo-forest-run/src/cache/draftHelpers.ts b/packages/apollo-forest-run/src/cache/draftHelpers.ts index fd37b3e40..eef99704f 100644 --- a/packages/apollo-forest-run/src/cache/draftHelpers.ts +++ b/packages/apollo-forest-run/src/cache/draftHelpers.ts @@ -2,6 +2,8 @@ import type { ChunkMatcher, ChunkProvider, CompositeListChunk, + CompositeListValue, + GraphValue, GraphValueReference, NodeChunk, ObjectChunk, @@ -21,6 +23,7 @@ import { isNodeValue, isObjectValue, findClosestNode, + getDataPathForDebugging, resolveGraphValueReference, retrieveEmbeddedValue, TraverseEnv, @@ -59,7 +62,9 @@ export function getObjectChunks( ); } -function* getEmbeddedObjectChunks( +// Exported for unit testing of the invariant message only (defensive guard, +// not reachable through the public cache API). +export function* getEmbeddedObjectChunks( pathEnv: TraverseEnv, nodeChunks: Iterable, ref: GraphValueReference, @@ -69,7 +74,18 @@ function* getEmbeddedObjectChunks( if (value === undefined || isMissingValue(value)) { continue; } - assert(isObjectValue(value) && value.key === false); + if (!isObjectValue(value) || value.key !== false) { + const at = embeddedPathClause(pathEnv, value); + const nodeType = chunk.type ? ` (in ${chunk.type})` : ""; + assert( + false, + `Failed to resolve embedded object in "${ + chunk.operation.debugName + }"${at}${nodeType}: expected an embedded object, got ${describeEmbeddedValue( + value, + )}`, + ); + } if (value.isAggregate) { for (const embeddedChunk of value.chunks) { yield embeddedChunk; @@ -80,6 +96,57 @@ function* getEmbeddedObjectChunks( } } +// Builds a " at path " clause for invariant messages. Wrapped in try/catch because +// it runs while reporting a violation, when the tree may already be inconsistent. +function embeddedPathClause(env: TraverseEnv, value: GraphValue): string { + if ( + (!isObjectValue(value) && !isCompositeListValue(value)) || + value.isAggregate + ) { + return ""; + } + try { + const path = getDataPathForDebugging(env, value); + return path.length ? ` at path ${path.join(".")}` : ""; + } catch { + return ""; + } +} + +function describeEmbeddedValue(value: GraphValue): string { + if (isCompositeListValue(value)) { + const itemType = listItemTypeName(value); + return itemType ? `a list of ${itemType}` : "a list"; + } + if (isObjectValue(value)) { + // An object value with a key is a normalized node, not an embedded object. + return value.key !== false + ? `a node (${value.type || "unknown type"})` + : "an object"; + } + return "a scalar"; +} + +// Best-effort __typename of the first typed item in a list, for diagnostics. +// Defensive: wrapped in try/catch and skips aggregates; returns undefined when no +// typed item is found (lists of plain objects/scalars carry no __typename). +function listItemTypeName(value: CompositeListValue): string | undefined { + if (value.isAggregate) { + return undefined; + } + try { + for (const item of value.itemChunks) { + const itemValue = item?.value; + if (itemValue && isObjectValue(itemValue) && itemValue.type) { + return itemValue.type; + } + } + } catch { + // ignore - diagnostics only + } + return undefined; +} + export function* getNodeChunks( layers: IndexedForest[], key: NodeKey, diff --git a/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts b/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts new file mode 100644 index 000000000..5f8d28170 --- /dev/null +++ b/packages/apollo-forest-run/src/forest/__tests__/updateObject.test.ts @@ -0,0 +1,134 @@ +import { replaceValue } from "../updateObject"; +import { UpdateTreeContext } from "../types"; +import { ForestRun } from "../../ForestRun"; +import { gql, createTestOperation } from "../../__tests__/helpers/descriptor"; +import { generateChunk } from "../../__tests__/helpers/values"; +import { resolveFieldValue } from "../../values/resolve"; +import { ObjectChunk } from "../../values/types"; + +describe("replaceValue invariant", () => { + it("reports the operation when a scalar is replaced with an object", () => { + const operation = createTestOperation(gql` + query ReplaceScalar { + name + } + `); + const scalarBase = resolveFieldValue( + generateChunk(`query ScalarSource { name }`).value, + "name", + ); + const objectReplacement = resolveFieldValue( + generateChunk( + `query ObjectSource { node { __typename @mock(value: "Widget") id } }`, + ).value, + "node", + ) as ObjectChunk; + + const context = { + operation, + findParent: () => undefined, + } as unknown as UpdateTreeContext; + + expect(() => + replaceValue(context, scalarBase as any, objectReplacement), + ).toThrow( + 'Invariant violation: Failed to update "query ReplaceScalar": cannot replace Scalar with Object (of type Widget)', + ); + }); +}); + +const nestedFeedListQuery = gql` + query BaseFeedQuery { + objectContainer { + __typename + id + items { + __typename + id + value { + note + } + } + } + listContainer { + __typename + id + items { + __typename + id + value { + note + } + } + } + } +`; +const nestedFeedObjectQuery = gql` + query ModelFeedQuery { + objectContainer { + __typename + id + items { + __typename + id + value { + note + } + } + } + } +`; + +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"; + +describe("incompatible object difference invariant", () => { + it("reports operation and path for a field two levels deep below a list", () => { + const cache = new ForestRun(); + + // The same Entity ("Entity:1") is reached through two different containers within + // a single tree: under "objectContainer" its "value" is an object, while under + // "listContainer" the same field is a list. This leaves the normalized entity with + // inconsistent structural representations of "value". + 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: [{ note: "old" }] }], + }, + }, + }); + + 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_OBJECT_DIFF_ERROR, + ); + }); +}); diff --git a/packages/apollo-forest-run/src/forest/__tests__/updateTree.test.ts b/packages/apollo-forest-run/src/forest/__tests__/updateTree.test.ts index 2fbbb3a3b..bd7ffbb4a 100644 --- a/packages/apollo-forest-run/src/forest/__tests__/updateTree.test.ts +++ b/packages/apollo-forest-run/src/forest/__tests__/updateTree.test.ts @@ -23,6 +23,7 @@ import { diffTree, GraphDifference } from "../../diff/diffTree"; import { cloneDeep, maybeDeepFreeze } from "@apollo/client/utilities"; import { DiffEnv } from "../../diff/types"; import { isCompositeListEntryTuple } from "../../values/predicates"; +import * as DifferenceKind from "../../diff/differenceKind"; test("no-op when there are no changes", () => { const before = completeObject(); @@ -1974,6 +1975,58 @@ describe("ApolloCompat: orphan nodes", () => { }); }); +describe("invariant diagnostics", () => { + const env: ForestEnv = { objectKey: (o: any) => `${o.__typename}:${o.id}` }; + + test("reports the operation when the tree has no single root chunk", () => { + const op = createTestOperation(gql` + query RootChunkTest { + node { + __typename + id + } + } + `); + const base = createTestTree(op, { + node: { __typename: "Node", id: "1" }, + } as any); + base.nodes.set(base.rootNodeKey, []); + + expect(() => updateTree(env, base, new Map())).toThrow( + 'Invariant violation: Failed to update "query RootChunkTest": expected a single root chunk, got 0', + ); + }); + + test("reports operation, path and type when a difference is missing for an affected chunk", () => { + const op = createTestOperation(gql` + query MissingDifferenceTest { + node { + __typename + id + } + } + `); + const base = createTestTree(op, { + node: { __typename: "Node", id: "1" }, + } as any); + + // White-box: alias the node chunk under a "ghost" key that has no matching + // chunk.key, so resolveAffectedChunks yields a chunk absent from the map. + const nodeKey = [...base.nodes.keys()].find( + (key) => key !== base.rootNodeKey, + ); + const nodeChunks = base.nodes.get(nodeKey as string); + base.nodes.set("GHOST", nodeChunks as any); + const differenceMap = new Map([ + ["GHOST", { kind: DifferenceKind.Replacement }], + ]); + + expect(() => updateTree(env, base, differenceMap)).toThrow( + 'Invariant violation: Failed to update "query MissingDifferenceTest" at path node: missing difference for Node', + ); + }); +}); + type TestValue = [SourceObject, DocumentNode, VariableValues?]; function diffAndUpdate( diff --git a/packages/apollo-forest-run/src/forest/updateObject.ts b/packages/apollo-forest-run/src/forest/updateObject.ts index 24bad6564..0bf8841cb 100644 --- a/packages/apollo-forest-run/src/forest/updateObject.ts +++ b/packages/apollo-forest-run/src/forest/updateObject.ts @@ -9,6 +9,7 @@ import type { ObjectChunk, ObjectDraft, ObjectValue, + ParentLocator, SourceCompositeList, SourceLeafValue, SourceNull, @@ -165,30 +166,24 @@ function updateValue( return replaceValue(context, base, difference.newValue); case DifferenceKind.ObjectDifference: { - assert( - Value.isObjectValue(base), - incompatibleDifferenceMessage( - context, - base, - difference, - location, - "Object", - ), - ); + // Note: building the diagnostic message is expensive (path/type lookups), so it is + // constructed only on the failure path rather than eagerly passed to assert(). + if (!Value.isObjectValue(base)) { + assert( + false, + incompatibleDifferenceMessage(context, base, difference, location), + ); + } return updateObjectValue(context, base, difference); } case DifferenceKind.CompositeListDifference: { - assert( - Value.isCompositeListValue(base), - incompatibleDifferenceMessage( - context, - base, - difference, - location, - "CompositeList", - ), - ); + if (!Value.isCompositeListValue(base)) { + assert( + false, + incompatibleDifferenceMessage(context, base, difference, location), + ); + } return updateCompositeListValue(context, base, difference); } @@ -210,40 +205,155 @@ function incompatibleDifferenceMessage( base: GraphChunk, difference: ObjectDifference | CompositeListDifference, location: UpdateValueLocation, - expectedKind: "Object" | "CompositeList", ): string { - const locationDescription = - location.kind === "field" - ? `field "${location.fieldName}"` - : `list item at index ${location.index}`; const differenceKind = Difference.isObjectDifference(difference) ? "ObjectDifference" : "CompositeListDifference"; - const differenceState = Difference.isObjectDifference(difference) - ? `dirty fields=${difference.dirtyFields?.size ?? 0}, pending fields=${ - difference.fieldQueue.size - }, field states=${difference.fieldState.size}` - : `dirty items=${difference.dirtyItems?.size ?? 0}, pending items=${ - difference.itemQueue.size - }, item states=${difference.itemState.size}, layout change=${Boolean( - difference.layout, - )}`; - - return ( - `cannot apply ${differenceKind} to ${valueKindName( - base, - )} at ${locationDescription} ` + - `while updating a ${context.operation.definition.operation} operation; ` + - `expected value kind=${expectedKind}, actual value kind=${valueKindName( + const typeClause = incomingTypeClause(differenceTypeName(difference)); + return updateFailureMessage( + context, + base, + `expected ${valueKindName(base)}, got ${differenceKind}${typeClause}`, + location, + ); +} + +function incompatibleReplacementMessage( + context: UpdateTreeContext, + base: GraphChunk, + replacement: GraphValue, + replacementKind: "Object" | "CompositeList", +): string { + const typeClause = incomingTypeClause(graphValueTypeName(replacement)); + return updateFailureMessage( + context, + base, + `cannot replace ${valueKindName( base, - )}; ` + - `difference metadata: ${differenceState}. ` + - `This indicates inconsistent structural representations for the same normalized object. ` + - `Likely causes include inconsistent response shapes for a repeated normalized entity or a cache-key collision. ` + - `Object keys, operation names, aliases, arguments, variables, type names, and cached values are omitted to protect privacy.` + )} with ${replacementKind}${typeClause}`, ); } +// Best-effort " (of type )" clause naming the __typename of the value being +// written, for diagnostics. Returns "" when no typename is available (plain embedded +// objects, lists and scalars carry no __typename). +function incomingTypeClause(typeName: string | undefined): string { + return typeName ? ` (of type ${typeName})` : ""; +} + +// __typename of an incoming value, when it is a typed object/node. +// Defensive: the tree may be inconsistent when an invariant fires, so wrapped in try/catch. +function graphValueTypeName(value: GraphValue): string | undefined { + try { + if (Value.isObjectValue(value)) { + return value.type || undefined; + } + } catch { + // ignore - diagnostics only + } + return undefined; +} + +// Best-effort __typename of the value an incompatible difference is trying to write, +// for diagnostics. For object differences it is recovered from a dirty "__typename" +// field (present only when the typename itself changed); for list differences it is the +// type of the first typed item being inserted. Defensive and diagnostics-only. +function differenceTypeName( + difference: ObjectDifference | CompositeListDifference, +): string | undefined { + try { + if (difference.kind === DifferenceKind.ObjectDifference) { + const entry = difference.fieldState?.get("__typename"); + const state = (Array.isArray(entry) ? entry[0] : entry)?.state; + if ( + state && + (state.kind === DifferenceKind.Replacement || + state.kind === DifferenceKind.Filler) + ) { + return typeof state.newValue === "string" ? state.newValue : undefined; + } + return undefined; + } + if (difference.kind === DifferenceKind.CompositeListDifference) { + for (const item of difference.layout ?? EMPTY_ARRAY) { + if (item && typeof item === "object" && Value.isObjectValue(item)) { + if (item.type) { + return item.type; + } + } + } + } + } catch { + // ignore - diagnostics only + } + return undefined; +} + +function updateFailureMessage( + context: UpdateTreeContext, + base: GraphChunk, + detail: string, + 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); + return pathString + ? `${prefix} at path ${pathString}${nodeClause}: ${detail}` + : `${prefix}${nodeClause}: ${detail}`; +} + +function describeLocation(location: UpdateValueLocation): string { + return location.kind === "field" + ? location.fieldName + : 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). +function nodeTypeClause( + env: { findParent: ParentLocator }, + chunk: GraphChunk, +): string { + if ( + (!Value.isObjectValue(chunk) && !Value.isCompositeListValue(chunk)) || + chunk.isAggregate + ) { + return ""; + } + try { + const node = Value.findClosestNode(chunk, env.findParent); + return node.type ? ` (in ${node.type})` : ""; + } catch { + return ""; + } +} + +// 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 +// be in an inconsistent state and path resolution could throw. +function chunkPath( + env: { findParent: ParentLocator }, + chunk: GraphChunk, +): (string | number)[] | undefined { + if (!Value.isObjectValue(chunk) && !Value.isCompositeListValue(chunk)) { + return undefined; + } + try { + return Value.getDataPathForDebugging(env, chunk); + } catch { + return undefined; + } +} + function valueKindName(value: GraphChunk): string { if (Value.isScalarValue(value)) { return "Scalar"; @@ -395,7 +505,9 @@ function updateCompositeListValue( return copy ?? base.data; } -function replaceValue( +// Exported for unit testing of the invariant message only (defensive guard, +// not reachable through the public cache API). +export function replaceValue( context: UpdateTreeContext, base: GraphChunk, replacement: GraphValue, @@ -408,15 +520,30 @@ function replaceValue( } switch (replacement.kind) { case ValueKind.Object: { - assert(Value.isCompositeValue(base)); + if (!Value.isCompositeValue(base)) { + assert( + false, + incompatibleReplacementMessage(context, base, replacement, "Object"), + ); + } return replaceObject(context, replacement, base.possibleSelections); } case ValueKind.CompositeList: { - assert( - Value.isCompositeListValue(base) || - Value.isCompositeNullValue(base) || - Value.isCompositeUndefinedValue(base), - ); + if ( + !Value.isCompositeListValue(base) && + !Value.isCompositeNullValue(base) && + !Value.isCompositeUndefinedValue(base) + ) { + assert( + false, + incompatibleReplacementMessage( + context, + base, + replacement, + "CompositeList", + ), + ); + } return replaceCompositeList(context, base, replacement); } diff --git a/packages/apollo-forest-run/src/forest/updateTree.ts b/packages/apollo-forest-run/src/forest/updateTree.ts index 90e92c566..fd873a0d7 100644 --- a/packages/apollo-forest-run/src/forest/updateTree.ts +++ b/packages/apollo-forest-run/src/forest/updateTree.ts @@ -9,6 +9,7 @@ import type { NodeKey, NodeChunk, CompositeValueChunk, + CompositeListChunk, ObjectDraft, SourceObject, } from "../values/types"; @@ -26,6 +27,7 @@ import { isDirty } from "../diff/difference"; import { createDraft, createParentLocator, + getDataPathForDebugging, hydrateDraft, isNodeValue, isObjectValue, @@ -33,6 +35,7 @@ import { isRootRef, isSourceObject, resolveObjectKey, + TraverseEnv, } from "../values"; import { updateObject } from "./updateObject"; import { createUpdateLogger } from "../telemetry/updateStats/updateLogger"; @@ -48,7 +51,14 @@ export function updateTree( getNodeChunks?: (key: NodeKey) => Iterable, ): UpdateTreeResult { const rootChunks = base.nodes.get(base.rootNodeKey); - assert(rootChunks?.length === 1); + if (rootChunks?.length !== 1) { + assert( + false, + `Failed to update "${ + base.operation.debugName + }": expected a single root chunk, got ${rootChunks?.length ?? 0}`, + ); + } const rootChunk = rootChunks[0]; const context: UpdateTreeContext = { operation: base.operation, @@ -102,7 +112,15 @@ export function updateTree( for (const chunk of chunkQueue) { const difference = differenceMap.get(chunk.key as string); - assert(difference); + if (!difference) { + const at = debugPathClause(context, chunk); + assert( + difference, + `Failed to update "${ + base.operation.debugName + }"${at}: missing difference for ${chunk.type || "object"}`, + ); + } statsLogger?.startChunkUpdate(chunk); const result = updateObject(context, chunk, difference); @@ -192,6 +210,20 @@ function resolveAffectedChunks( return affectedChunks; } +// Builds a " at path " clause for invariant messages. Wrapped in try/catch because +// it runs while reporting a violation, when the tree may already be inconsistent. +function debugPathClause( + env: TraverseEnv, + chunk: ObjectChunk | CompositeListChunk, +): string { + try { + const path = getDataPathForDebugging(env, chunk); + return path.length ? ` at path ${path.join(".")}` : ""; + } catch { + return ""; + } +} + function completeObject( env: ForestEnv, base: IndexedTree, diff --git a/packages/apollo-forest-run/src/values/__tests__/traverse.test.ts b/packages/apollo-forest-run/src/values/__tests__/traverse.test.ts index 19d9a17e7..eb53215df 100644 --- a/packages/apollo-forest-run/src/values/__tests__/traverse.test.ts +++ b/packages/apollo-forest-run/src/values/__tests__/traverse.test.ts @@ -690,4 +690,34 @@ describe(retrieveEmbeddedValue, () => { expect(result).toBeUndefined(); }); + + it("throws a descriptive invariant when the reference does not descend from the source node", () => { + const query = `query DescendTest { + nodeA { + __typename @mock(value: "NodeA") + id @mock(value: "A") + child { + name + } + } + nodeB { + __typename @mock(value: "NodeB") + id @mock(value: "B") + } + }`; + const { value: root, dataMap } = generateChunk(query); + const env = { findParent: createParentLocator(dataMap) }; + + const nodeA = resolveFieldValue(root, "nodeA") as NodeChunk; + const childA = resolveFieldValue(nodeA, "child") as ObjectChunk; + const nodeB = resolveFieldValue(root, "nodeB") as NodeValue; + + // A reference pointing into "nodeA" cannot be resolved against "nodeB". + const childRef = getGraphValueReference(env, childA); + const run = () => retrieveEmbeddedValue(env, nodeB, childRef); + + expect(run).toThrow( + 'Invariant violation: Failed to resolve embedded value in "query DescendTest" at path nodeB: reference does not descend from NodeB', + ); + }); }); diff --git a/packages/apollo-forest-run/src/values/traverse.ts b/packages/apollo-forest-run/src/values/traverse.ts index b6a9bd23e..a55a2b785 100644 --- a/packages/apollo-forest-run/src/values/traverse.ts +++ b/packages/apollo-forest-run/src/values/traverse.ts @@ -51,6 +51,20 @@ export function getDataPathForDebugging( return path; } +// Builds a " at path " clause for invariant messages. Wrapped in try/catch because +// it runs while reporting a violation, when the tree may already be inconsistent. +function embeddedPathClause(env: TraverseEnv, source: NodeValue): string { + if (source.isAggregate) { + return ""; + } + try { + const path = getDataPathForDebugging(env, source); + return path.length ? ` at path ${path.join(".")}` : ""; + } catch { + return ""; + } +} + export function findClosestNode( chunk: ObjectChunk | CompositeListChunk, findParent: ParentLocator, @@ -222,11 +236,23 @@ export function retrieveEmbeddedValue( ); parentRef = refParentLocator(parentRef.parent); } - assert( - parentRef && - Predicates.isParentObjectRef(parentRef) && - parentRef.parent.key === source.key, - ); + if ( + !parentRef || + !Predicates.isParentObjectRef(parentRef) || + parentRef.parent.key !== source.key + ) { + const operationClause = + "operation" in source ? ` in "${source.operation.debugName}"` : ""; + assert( + false, + `Failed to resolve embedded value${operationClause}${embeddedPathClause( + env, + source, + )}: reference does not descend from ${ + source.type || "the expected node" + }`, + ); + } if (source === resolveGraphValueReference(parentRef)) { return resolveGraphValueReference(ref[0]); }