fix: fail gracefully during restore (#10673)

* fix: fail gracefully during restore

* tests

packages/excalidraw/data/restore.ts

@@ -136,99 +136,103 @@ const repairBinding = <T extends ExcalidrawArrowElement>(
   existingElementsMap: Readonly<ElementsMap> | null | undefined,
   startOrEnd: "start" | "end",
 ): FixedPointBinding | null => {
-  if (!binding) {
-    return null;
-  }
-
-  // ---------------------------------------------------------------------------
-  // elbow arrows
-  // ---------------------------------------------------------------------------
-
-  if (isElbowArrow(element)) {
-    const fixedPointBinding:
-      | ExcalidrawElbowArrowElement["startBinding"]
-      | ExcalidrawElbowArrowElement["endBinding"] = {
-      ...binding,
-      fixedPoint: normalizeFixedPoint(binding.fixedPoint ?? [0, 0]),
-      mode: binding.mode || "orbit",
-    };
-
-    return fixedPointBinding;
-  }
-
-  // ---------------------------------------------------------------------------
-  // simple arrows
-  // ---------------------------------------------------------------------------
-
-  // binding schema v2
-  // ---------------------------------------------------------------------------
-
-  if (binding.mode) {
-    // if latest binding schema, don't check if binding.elementId exists
-    // (it's done in a separate pass)
-    if (binding.elementId) {
-      return {
-        elementId: binding.elementId,
-        mode: binding.mode,
-        fixedPoint: normalizeFixedPoint(binding.fixedPoint || [0.5, 0.5]),
-      } as FixedPointBinding | null;
+  try {
+    if (!binding) {
+      return null;
     }
-    return null;
+
+    // ---------------------------------------------------------------------------
+    // elbow arrows
+    // ---------------------------------------------------------------------------
+
+    if (isElbowArrow(element)) {
+      const fixedPointBinding:
+        | ExcalidrawElbowArrowElement["startBinding"]
+        | ExcalidrawElbowArrowElement["endBinding"] = {
+        ...binding,
+        fixedPoint: normalizeFixedPoint(binding.fixedPoint ?? [0, 0]),
+        mode: binding.mode || "orbit",
+      };
+
+      return fixedPointBinding;
+    }
+
+    // ---------------------------------------------------------------------------
+    // simple arrows
+    // ---------------------------------------------------------------------------
+
+    // binding schema v2
+    // ---------------------------------------------------------------------------
+
+    if (binding.mode) {
+      // if latest binding schema, don't check if binding.elementId exists
+      // (it's done in a separate pass)
+      if (binding.elementId) {
+        return {
+          elementId: binding.elementId,
+          mode: binding.mode,
+          fixedPoint: normalizeFixedPoint(binding.fixedPoint || [0.5, 0.5]),
+        } as FixedPointBinding | null;
+      }
+      return null;
+    }
+
+    // binding schema v1 (legacy) -> attempt to migrate to v2
+    // ---------------------------------------------------------------------------
+
+    const targetBoundElement =
+      (targetElementsMap.get(binding.elementId) as ExcalidrawBindableElement) ||
+      undefined;
+    const boundElement =
+      targetBoundElement ||
+      (existingElementsMap?.get(
+        binding.elementId,
+      ) as ExcalidrawBindableElement) ||
+      undefined;
+    const elementsMap = targetBoundElement
+      ? targetElementsMap
+      : existingElementsMap;
+
+    // migrating legacy focus point bindings
+    if (boundElement && elementsMap) {
+      const p = LinearElementEditor.getPointAtIndexGlobalCoordinates(
+        element,
+        startOrEnd === "start" ? 0 : element.points.length - 1,
+        elementsMap,
+      );
+      const mode = isPointInElement(p, boundElement, elementsMap)
+        ? "inside"
+        : "orbit";
+      const focusPoint =
+        mode === "inside"
+          ? p
+          : projectFixedPointOntoDiagonal(
+              element,
+              p,
+              boundElement,
+              startOrEnd,
+              elementsMap,
+            ) || p;
+      const { fixedPoint } = calculateFixedPointForNonElbowArrowBinding(
+        element,
+        boundElement,
+        startOrEnd,
+        elementsMap,
+        focusPoint,
+      );
+
+      return {
+        mode,
+        elementId: binding.elementId,
+        fixedPoint,
+      };
+    }
+
+    console.error(`could not repair binding for element`);
+  } catch (error) {
+    console.error("Error repairing binding:", error);
   }
 
-  // binding schema v1 (legacy) -> attempt to migrate to v2
-  // ---------------------------------------------------------------------------
-
-  const targetBoundElement =
-    (targetElementsMap.get(binding.elementId) as ExcalidrawBindableElement) ||
-    undefined;
-  const boundElement =
-    targetBoundElement ||
-    (existingElementsMap?.get(
-      binding.elementId,
-    ) as ExcalidrawBindableElement) ||
-    undefined;
-  const elementsMap = targetBoundElement
-    ? targetElementsMap
-    : existingElementsMap;
-
-  // migrating legacy focus point bindings
-  if (boundElement && elementsMap) {
-    const p = LinearElementEditor.getPointAtIndexGlobalCoordinates(
-      element,
-      startOrEnd === "start" ? 0 : element.points.length - 1,
-      elementsMap,
-    );
-    const mode = isPointInElement(p, boundElement, elementsMap)
-      ? "inside"
-      : "orbit";
-    const focusPoint =
-      mode === "inside"
-        ? p
-        : projectFixedPointOntoDiagonal(
-            element,
-            p,
-            boundElement,
-            startOrEnd,
-            elementsMap,
-          ) || p;
-    const { fixedPoint } = calculateFixedPointForNonElbowArrowBinding(
-      element,
-      boundElement,
-      startOrEnd,
-      elementsMap,
-      focusPoint,
-    );
-
-    return {
-      mode,
-      elementId: binding.elementId,
-      fixedPoint,
-    };
-  }
-
-  console.error(`could not repair binding for element`);
-
   return null;
 };
 
@@ -639,15 +643,20 @@ export const restoreElements = <T extends ExcalidrawElement>(
       if (element.type === "selection") {
         return elements;
       }
-
-      let migratedElement: ExcalidrawElement | null = restoreElement(
-        element,
-        targetElementsMap,
-        existingElementsMap,
-        {
-          deleteInvisibleElements: opts?.deleteInvisibleElements,
-        },
-      );
+      let migratedElement: ExcalidrawElement | null;
+      try {
+        migratedElement = restoreElement(
+          element,
+          targetElementsMap,
+          existingElementsMap,
+          {
+            deleteInvisibleElements: opts?.deleteInvisibleElements,
+          },
+        );
+      } catch (error) {
+        console.error("Error restoring element:", error);
+        migratedElement = null;
+      }
       if (migratedElement) {
         const localElement = existingElementsMap?.get(element.id);
 

packages/excalidraw/tests/data/__snapshots__/restore.test.ts.snap

@@ -1,5 +1,52 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
+exports[`repairing bindings > should strip arrow binding if repair throws 1`] = `
+{
+  "angle": 0,
+  "backgroundColor": "transparent",
+  "boundElements": [],
+  "customData": undefined,
+  "elbowed": false,
+  "endArrowhead": null,
+  "endBinding": null,
+  "fillStyle": "solid",
+  "frameId": null,
+  "groupIds": [],
+  "height": 100,
+  "id": "id-arrow01",
+  "index": "a0",
+  "isDeleted": false,
+  "link": null,
+  "locked": false,
+  "opacity": 100,
+  "points": [
+    [
+      0,
+      0,
+    ],
+    [
+      100,
+      100,
+    ],
+  ],
+  "roughness": 1,
+  "roundness": null,
+  "seed": Any<Number>,
+  "startArrowhead": null,
+  "startBinding": null,
+  "strokeColor": "#1e1e1e",
+  "strokeStyle": "solid",
+  "strokeWidth": 2,
+  "type": "arrow",
+  "updated": 1,
+  "version": 2,
+  "versionNonce": Any<Number>,
+  "width": 100,
+  "x": 0,
+  "y": 0,
+}
+`;
+
 exports[`restoreElements > should restore arrow element correctly 1`] = `
 {
   "angle": 0,
@@ -381,3 +428,50 @@ exports[`restoreElements > should restore text element correctly with unknown fo
   "y": 0,
 }
 `;
+
+exports[`restoreElements > should strip arrow binding if repair throws 1`] = `
+{
+  "angle": 0,
+  "backgroundColor": "transparent",
+  "boundElements": [],
+  "customData": undefined,
+  "elbowed": false,
+  "endArrowhead": null,
+  "endBinding": null,
+  "fillStyle": "solid",
+  "frameId": null,
+  "groupIds": [],
+  "height": 100,
+  "id": "id-arrow01",
+  "index": "a0",
+  "isDeleted": false,
+  "link": null,
+  "locked": false,
+  "opacity": 100,
+  "points": [
+    [
+      0,
+      0,
+    ],
+    [
+      100,
+      100,
+    ],
+  ],
+  "roughness": 1,
+  "roundness": null,
+  "seed": Any<Number>,
+  "startArrowhead": null,
+  "startBinding": null,
+  "strokeColor": "#1e1e1e",
+  "strokeStyle": "solid",
+  "strokeWidth": 2,
+  "type": "arrow",
+  "updated": 1,
+  "version": 2,
+  "versionNonce": Any<Number>,
+  "width": 100,
+  "x": 0,
+  "y": 0,
+}
+`;

packages/excalidraw/tests/data/restore.test.ts

@@ -200,6 +200,36 @@ describe("restoreElements", () => {
     });
   });
 
+  it("should strip element if restore fails", () => {
+    const rect1 = API.createElement({
+      type: "rectangle",
+      boundElements: [],
+    });
+    const rect2 = API.createElement({
+      type: "rectangle",
+      boundElements: [],
+    });
+
+    // define getter for a property we access during restoreElement that throws
+    Object.defineProperty(rect2, "seed", {
+      get: () => {
+        throw new Error("FORBIDDEN!");
+      },
+    });
+
+    const restoredElements = restore.restoreElements([rect1, rect2], null);
+
+    expect(restoredElements.length).toBe(1);
+
+    expect(restoredElements).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          id: rect1.id,
+        }),
+      ]),
+    );
+  });
+
   it("should remove imperceptibly small elements", () => {
     const arrowElement = API.createElement({
       type: "arrow",
@@ -717,6 +747,58 @@ describe("restoreAppState", () => {
 });
 
 describe("repairing bindings", () => {
+  it("should strip arrow binding if repair throws", () => {
+    const container = API.createElement({
+      type: "rectangle",
+      boundElements: [],
+    });
+    const arrowElement = API.createElement({
+      type: "arrow",
+      id: "id-arrow01",
+      endBinding: {
+        elementId: container.id,
+        fixedPoint: [0.5, 0.5],
+        mode: "inside",
+      },
+    });
+
+    Object.assign(container, {
+      boundElements: [{ type: "arrow", id: arrowElement.id }],
+    });
+    // make invalid binding -> throws during repair
+    Object.assign(arrowElement, {
+      endBinding: {
+        // invalid reference
+        elementId: 42,
+      },
+    });
+
+    const restoredElements = restore.restoreElements(
+      [arrowElement, container],
+      null,
+    );
+
+    expect(restoredElements).toEqual(
+      expect.arrayContaining([
+        expect.objectContaining({
+          id: arrowElement.id,
+          endBinding: null,
+        }),
+        expect.objectContaining({
+          id: container.id,
+        }),
+      ]),
+    );
+
+    const restoredArrow = restoredElements[0] as ExcalidrawLinearElement;
+
+    expect(restoredArrow).toMatchSnapshot({
+      seed: expect.any(Number),
+      versionNonce: expect.any(Number),
+      // endBinding: expect.any(Object),
+    });
+  });
+
   it("should repair container boundElements when repair is true", () => {
     const container = API.createElement({
       type: "rectangle",