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 fdbb4e591..ebe407336 100644 --- a/packages/apollo-forest-run/src/__tests__/regression.test.ts +++ b/packages/apollo-forest-run/src/__tests__/regression.test.ts @@ -1086,6 +1086,131 @@ 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: 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; + 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_KEY_COLLISION); +}); + test("matches apollo InMemoryCache behavior on incorrect cache overwrites", () => { const listQuery = gql` query ListQuery { 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 706ac5ced..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, @@ -98,7 +99,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 +151,39 @@ 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)); + // 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)); + if (!Value.isCompositeListValue(base)) { + assert( + false, + incompatibleDifferenceMessage(context, base, difference, location), + ); + } return updateCompositeListValue(context, base, difference); } @@ -179,6 +200,189 @@ function updateValue( } } +function incompatibleDifferenceMessage( + context: UpdateTreeContext, + base: GraphChunk, + difference: ObjectDifference | CompositeListDifference, + location: UpdateValueLocation, +): string { + const differenceKind = Difference.isObjectDifference(difference) + ? "ObjectDifference" + : "CompositeListDifference"; + 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, + )} 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"; + } + 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 +406,7 @@ function updateCompositeListValue( context, Value.resolveListItemChunk(base, index), itemDiff, + { kind: "listItem", index }, ); if (updatedValue === base.data[index]) { continue; @@ -300,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, @@ -313,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]); }