Report full data path in incompatible-update invariant messages

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"
+}

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`
@@ -83,6 +118,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 +170,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);
+  });
 });

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,85 @@ 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 {
+  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;
+  }
+}
+
 function describeLocation(location: UpdateValueLocation): string {
   return location.kind === "field"
-    ? location.fieldName
+    ? location.field.dataKey
     : String(location.index);
 }
 
 // Best-effort " (in <TypeName>)" 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 +446,7 @@ function updateCompositeListValue(
       context,
       Value.resolveListItemChunk(base, index),
       itemDiff,
-      { kind: "listItem", index },
+      { kind: "listItem", parent: base, index },
     );
     if (updatedValue === base.data[index]) {
       continue;