#3372: Show full error cause list in page editor Log (#3614)

Co-authored-by: Todd Schiller <todd.schiller@gmail.com>

src/components/errors/__snapshots__/getErrorDetails.test.tsx.snap

@@ -575,7 +575,7 @@ exports[`Template render error 1`] = `
     <div
       class="col"
     >
-      Template render error: (unknown path) [Line 1, Column 3]
+      (unknown path) [Line 1, Column 3]
   parseAggregate: expected colon after dict key
     </div>
   </div>

src/components/errors/getErrorDetails.tsx

@@ -17,33 +17,22 @@
 
 import React from "react";
 import { InputValidationError, OutputValidationError } from "@/blocks/errors";
-import { selectSpecificError } from "@/errors/errorHelpers";
+import {
+  getErrorMessageWithCauses,
+  selectSpecificError,
+} from "@/errors/errorHelpers";
 import { ErrorObject } from "serialize-error";
 import InputValidationErrorDetail from "./InputValidationErrorDetail";
 import NetworkErrorDetail from "./NetworkErrorDetail";
 import OutputValidationErrorDetail from "./OutputValidationErrorDetail";
 import { Col, Row } from "react-bootstrap";
-import { UnknownObject } from "@/types";
 import { ClientRequestError } from "@/errors/clientRequestErrors";
-import { isObject } from "@/utils";
 
 type ErrorDetails = {
   title: string;
   detailsElement: React.ReactElement;
 };
 
-/**
- * Find the root cause
- * @deprecated Look for specific errors via selectSpecificError
- */
-function getRootCause(error: unknown): unknown {
-  while (isObject(error) && error.cause != null) {
-    error = error.cause;
-  }
-
-  return error;
-}
-
 export default function getErrorDetails(error: ErrorObject): ErrorDetails {
   const inputValidationError = selectSpecificError(error, InputValidationError);
   if (inputValidationError) {
@@ -76,17 +65,11 @@ export default function getErrorDetails(error: ErrorObject): ErrorDetails {
     };
   }
 
-  // TODO: Use getErrorMessage instead, or something that combines the whole error cause stack
-  const { name = "Error", message = "Unknown error" } = (getRootCause(error) ??
-    {}) as UnknownObject;
-
   return {
     title: "Error",
     detailsElement: (
       <Row>
-        <Col>
-          {name}: {message}
-        </Col>
+        <Col>{getErrorMessageWithCauses(error)}</Col>
       </Row>
     ),
   };

src/components/logViewer/LogTable.stories.tsx

@@ -79,6 +79,22 @@ const ERROR_MESSAGE: LogEntry = {
   error: serializeError(new Error("Simple error")),
 };
 
+const NESTED_ERROR_MESSAGE: LogEntry = {
+  uuid: uuidv4(),
+  timestamp: Date.now().toString(),
+  message: "Sample error with cause chain",
+  level: "error",
+  context: {
+    // Just the context that will show up in the table
+    blockId,
+  },
+  error: serializeError(
+    new Error("Simple error", {
+      cause: new Error("Cause error", { cause: new Error("Cause error #2") }),
+    })
+  ),
+};
+
 const sampleSchema: Schema = {
   type: "object",
   properties: {
@@ -124,5 +140,10 @@ const CONTEXT_ERROR_MESSAGE: LogEntry = {
 export const Populated = Template.bind({});
 Populated.args = {
   hasEntries: true,
-  pageEntries: [DEBUG_MESSAGE, ERROR_MESSAGE, CONTEXT_ERROR_MESSAGE],
+  pageEntries: [
+    DEBUG_MESSAGE,
+    ERROR_MESSAGE,
+    NESTED_ERROR_MESSAGE,
+    CONTEXT_ERROR_MESSAGE,
+  ],
 };

src/errors/errorHelpers.test.ts

@@ -17,6 +17,7 @@
 
 import {
   getErrorMessage,
+  getErrorMessageWithCauses,
   hasSpecificErrorCause,
   IGNORED_ERROR_PATTERNS,
   isErrorObject,
@@ -182,6 +183,66 @@ describe("getErrorMessage", () => {
   });
 });
 
+describe("getErrorMessageWithCauses", () => {
+  const FIRST_ERROR = "There was an error while fetching the page";
+  const SECOND_ERROR = "Maybe you are not connected to the internet?";
+  const THIRD_ERROR = "The network request failed (NO_NETWORK)";
+  test("handles string", () => {
+    expect(getErrorMessageWithCauses(FIRST_ERROR)).toBe(FIRST_ERROR);
+  });
+
+  test("handles vanilla error", () => {
+    expect(getErrorMessageWithCauses(new Error(FIRST_ERROR))).toBe(FIRST_ERROR);
+  });
+
+  test("handles null/undefined", () => {
+    expect(getErrorMessageWithCauses(null)).toBe("Unknown error");
+    // eslint-disable-next-line unicorn/no-useless-undefined -- testing value since it comes from variable/expression in the wild
+    expect(getErrorMessageWithCauses(undefined)).toBe("Unknown error");
+  });
+
+  test("handles good causes", () => {
+    expect(
+      getErrorMessageWithCauses(
+        new Error(FIRST_ERROR, { cause: new Error(SECOND_ERROR) })
+      )
+    ).toMatchInlineSnapshot(`
+      "There was an error while fetching the page.
+      Maybe you are not connected to the internet?"
+    `);
+    expect(
+      getErrorMessageWithCauses(
+        new Error(FIRST_ERROR, {
+          cause: new Error(SECOND_ERROR, { cause: new Error(THIRD_ERROR) }),
+        })
+      )
+    ).toMatchInlineSnapshot(`
+      "There was an error while fetching the page.
+      Maybe you are not connected to the internet?
+      The network request failed (NO_NETWORK)."
+    `);
+  });
+
+  test("handles questionable causes", () => {
+    expect(
+      getErrorMessageWithCauses(new Error(FIRST_ERROR, { cause: null }))
+    ).toBe(FIRST_ERROR);
+    expect(
+      getErrorMessageWithCauses(new Error(FIRST_ERROR, { cause: undefined }))
+    ).toBe(FIRST_ERROR);
+    expect(getErrorMessageWithCauses(new Error(FIRST_ERROR, { cause: "idk" })))
+      .toMatchInlineSnapshot(`
+        "There was an error while fetching the page.
+        idk."
+      `);
+    expect(getErrorMessageWithCauses(new Error(FIRST_ERROR, { cause: 420 })))
+      .toMatchInlineSnapshot(`
+        "There was an error while fetching the page.
+        420."
+      `);
+  });
+});
+
 describe("isErrorObject", () => {
   test("handles error", () => {
     expect(isErrorObject(new Error(TEST_MESSAGE))).toBe(true);

src/errors/errorHelpers.ts

@@ -16,7 +16,7 @@
  */
 
 import { deserializeError, ErrorObject } from "serialize-error";
-import { isObject, matchesAnyPattern } from "@/utils";
+import { isObject, matchesAnyPattern, smartAppendPeriod } from "@/utils";
 import safeJsonStringify from "json-stringify-safe";
 import { truncate } from "lodash";
 import type { ContextError } from "@/errors/genericErrors";
@@ -212,6 +212,34 @@ export function getErrorMessage(
   return String(selectError(error).message ?? defaultMessage);
 }
 
+export function getErrorMessageWithCauses(
+  error: unknown,
+  defaultMessage = DEFAULT_ERROR_MESSAGE
+): string {
+  if (isErrorObject(error) && error.cause) {
+    return getErrorCauseList(error)
+      .map((error) => smartAppendPeriod(getErrorMessage(error)))
+      .join("\n");
+  }
+
+  // Handle cause-less messages more simply, they don't need to end with a period.
+  return getErrorMessage(error, defaultMessage);
+}
+
+/***
+ * Return chain of error causes (including the top-level error)
+ */
+export function getErrorCauseList(error: unknown): unknown[] {
+  const errors = [];
+
+  while (error != null) {
+    errors.push(error);
+    error = (error as Error)?.cause;
+  }
+
+  return errors;
+}
+
 /**
  * Handle ErrorEvents, i.e., generated from window.onerror
  * @param event the error event

src/utils.test.ts

@@ -23,6 +23,7 @@ import {
   matchesAnyPattern,
   makeURL,
   getScopeAndId,
+  smartAppendPeriod,
 } from "@/utils";
 import type { RegistryId, SafeString } from "@/core";
 import { BusinessError } from "@/errors/businessErrors";
@@ -229,3 +230,44 @@ describe("getScopeAndId", () => {
     expect(getScopeAndId(id)).toStrictEqual(["@foo", undefined]);
   });
 });
+
+describe("smartAppendPeriod", () => {
+  it("adds when missing", () => {
+    expect(smartAppendPeriod("add")).toBe("add.");
+
+    // After ])
+    expect(smartAppendPeriod("append (parens)")).toBe("append (parens).");
+    expect(smartAppendPeriod("append [bracket]")).toBe("append [bracket].");
+
+    // Before "'
+    expect(smartAppendPeriod("prepend 'apos'")).toBe("prepend 'apos.'");
+    expect(smartAppendPeriod('prepend "quotes"')).toBe('prepend "quotes."');
+  });
+
+  it("keeps it if present", () => {
+    const punctuation = ",.;:?!";
+    const strings = [
+      "keep",
+      "keep (parens)",
+      "keep [bracket]",
+      "keep 'apos'",
+      'keep "quotes"',
+    ];
+
+    for (const string of strings) {
+      for (const piece of punctuation) {
+        // Test trailing punctuation
+        expect(smartAppendPeriod(string + piece)).toBe(string + piece);
+
+        // Test punctuation to the left of )]"'
+        if (/\W$/.test(string)) {
+          const punctuationBeforeWrapper = [...string];
+          punctuationBeforeWrapper.splice(-1, 0, piece); // Add punctuation
+          expect(smartAppendPeriod(punctuationBeforeWrapper.join(""))).toBe(
+            punctuationBeforeWrapper.join("")
+          );
+        }
+      }
+    }
+  });
+});

src/utils.ts

@@ -574,3 +574,26 @@ export function getScopeAndId(
   const [scope, ...idParts] = split(value, "/");
   return [scope, idParts.join("/")];
 }
+
+const punctuation = [...".,;:?!"];
+
+/**
+ * Appends a period to a string as long as it doesn't end with one.
+ * Considers quotes and parentheses and it always trims the trailing spaces.
+ */
+export function smartAppendPeriod(string: string): string {
+  const trimmed = string.trimEnd();
+  const [secondLastChar, lastChar] = trimmed.slice(-2);
+  if (punctuation.includes(lastChar) || punctuation.includes(secondLastChar)) {
+    // Already punctuated
+    return trimmed;
+  }
+
+  // Else: No punctuation, find where to place it
+
+  if (lastChar === '"' || lastChar === "'") {
+    return trimmed.slice(0, -1) + "." + lastChar;
+  }
+
+  return trimmed + ".";
+}