Merge branch 'main' into feature/8070-AA-brick-errors

package.json

@@ -58,7 +58,7 @@
     "abort-utils": "^1.2.0",
     "ace-builds": "^1.33.1",
     "autocompleter": "^9.2.1",
-    "axios": "^0.27.2",
+    "axios": "^0.28.1",
     "batched-function": "^2.0.1",
     "bootstrap": "^4.6.0",
     "bootstrap-icons": "^1.11.3",

src/bricks/readers/ElementReader.test.ts

@@ -17,10 +17,19 @@
 
 import { ElementReader } from "@/bricks/readers/ElementReader";
 import { validateUUID } from "@/types/helpers";
+import { rectFactory } from "@/testUtils/factories/domFactories";
 
 const reader = new ElementReader();
 
 describe("ElementReader", () => {
+  beforeEach(() => {
+    // `jsdom` does not implement full layout engine
+    // https://github.com/jsdom/jsdom#unimplemented-parts-of-the-web-platform
+    (Element.prototype.getBoundingClientRect as any) = jest.fn(() =>
+      rectFactory(),
+    );
+  });
+
   test("it produces valid element reference", async () => {
     const div = document.createElement("div");
     const { ref } = await reader.read(div);
@@ -44,4 +53,29 @@ describe("ElementReader", () => {
     const { isVisible } = await reader.read(div);
     expect(isVisible).toBe(true);
   });
+
+  test("isInViewport: true for element in document", async () => {
+    const div = document.createElement("div");
+    div.innerHTML = "<p>Some text</p>";
+    document.body.append(div);
+
+    const { isInViewport } = await reader.read(div);
+    expect(isInViewport).toBe(true);
+  });
+
+  test("isInViewport: false for element partially outside of document", async () => {
+    (Element.prototype.getBoundingClientRect as any) = jest.fn(() =>
+      rectFactory({
+        width: window.innerWidth + 1,
+        right: window.innerWidth + 1,
+      }),
+    );
+
+    const div = document.createElement("div");
+    div.innerHTML = "<p>Some text</p>";
+    document.body.append(div);
+
+    const { isInViewport } = await reader.read(div);
+    expect(isInViewport).toBe(false);
+  });
 });

src/bricks/readers/ElementReader.ts

@@ -20,7 +20,7 @@ import { getReferenceForElement } from "@/contentScript/elementReference";
 import { ReaderABC } from "@/types/bricks/readerTypes";
 import { type SelectorRoot } from "@/types/runtimeTypes";
 import { type Schema } from "@/types/schemaTypes";
-import { isVisible } from "@/utils/domUtils";
+import { isInViewport, isVisible } from "@/utils/domUtils";
 
 /**
  * Read attributes, text, etc. from an HTML element.
@@ -49,6 +49,7 @@ export class ElementReader extends ReaderABC {
     return {
       ref: getReferenceForElement(element),
       isVisible: isVisible(element),
+      isInViewport: isInViewport(element),
       tagName: element.tagName,
       attrs: Object.fromEntries(
         Object.values(element.attributes).map((x) => [x.name, x.value]),
@@ -89,8 +90,20 @@ export class ElementReader extends ReaderABC {
         type: "boolean",
         description: "True if the element is visible",
       },
+      isInViewport: {
+        type: "boolean",
+        description: "True if element is completely in the viewport",
+      },
     },
-    required: ["tagName", "attrs", "data", "text", "ref", "isVisible"],
+    required: [
+      "tagName",
+      "attrs",
+      "data",
+      "text",
+      "ref",
+      "isVisible",
+      "isInViewport",
+    ],
     additionalProperties: false,
   };
 

src/components/errors/NetworkErrorDetail.tsx

@@ -55,7 +55,7 @@ const NetworkErrorDetail: React.FunctionComponent<{
   );
 
   const cleanConfig = useMemo(() => {
-    const { data, ...rest } = error.config;
+    const { data, ...rest } = error.config ?? {};
     return {
       ...rest,
       data: tryParse(data),

src/contrib/google/sheets/core/handleGoogleRequestRejection.ts

@@ -16,15 +16,11 @@
  */
 
 import { type SanitizedIntegrationConfig } from "@/integrations/integrationTypes";
-import {
-  getErrorMessage,
-  getRootCause,
-  selectError,
-} from "@/errors/errorHelpers";
+import { getErrorMessage, selectError } from "@/errors/errorHelpers";
 import { isObject } from "@/utils/objectUtils";
-import { isAxiosError } from "@/errors/networkErrorHelpers";
 import { deleteCachedAuthData } from "@/background/messenger/strict/api";
 import { type Nullishable } from "@/utils/nullishUtils";
+import { selectAxiosError } from "@/data/service/requestErrorUtils";
 
 class PermissionsError extends Error {
   override name = "PermissionsError";
@@ -41,11 +37,9 @@ export async function handleGoogleRequestRejection(
   error: unknown,
   googleAccount: Nullishable<SanitizedIntegrationConfig>,
 ): Promise<Error> {
-  // Request errors from proxyRequest are wrapped in ContextError which includes metadata about the integration
-  // configuration. Therefore, get root cause for determining if this is an Axios error
-  const rootCause = getRootCause(error);
-
-  console.debug("Error making Google request", { error });
+  console.debug("Error making Google request", {
+    error,
+  });
 
   if (!isObject(error)) {
     // Shouldn't happen in practice, but be defensive
@@ -54,12 +48,16 @@ export async function handleGoogleRequestRejection(
     });
   }
 
-  if (!isAxiosError(rootCause) || rootCause.response == null) {
+  // Request errors from proxyRequest are wrapped in ContextError which includes metadata about the integration
+  // configuration. Therefore, get axios error if it exists.
+  const axiosError = selectAxiosError(error);
+
+  if (axiosError?.response == null) {
     // It should always be an error-like object at this point, but be defensive.
     return selectError(error);
   }
 
-  const { status } = rootCause.response;
+  const { status } = axiosError.response;
 
   if ([403, 404].includes(status)) {
     const message =

src/data/service/errorService.ts

@@ -91,7 +91,7 @@ export async function selectExtraContext(
   const axiosError = selectAxiosError(error);
 
   if (
-    axiosError &&
+    axiosError?.config &&
     ((await flagOn("enterprise-telemetry")) ||
       (await isAppRequestError(axiosError)))
   ) {

src/utils/domUtils.ts

@@ -132,11 +132,40 @@ export async function waitForBody(): Promise<void> {
 
 /**
  * Return true if the element is visible (i.e. not in `display: none`), even if outside the viewport
+ * See https://developer.mozilla.org/en-US/docs/Web/API/Element/checkVisibility
+ *
+ * To determine if an element is fully contained within the viewport, use `isInViewport`.
+ *
+ * @see isInViewport
  */
 export function isVisible(element: HTMLElement): boolean {
   return element.checkVisibility();
 }
 
+/**
+ * Returns true if the element is completely in the viewport. The method does not consider the element's
+ * opacity, z-index, or other CSS properties that may affect visibility.
+ *
+ * Example scenarios where the method may return counterintuitive results:
+ * - Elements hidden by z-index
+ * - If they are hidden by overflow-scroll in element's container
+ * - Things hidden by relative/absolute positioning
+ * - Elements within an iframe that is not entirely visible in the top frame
+ *
+ * @see isVisible
+ */
+export function isInViewport(element: HTMLElement): boolean {
+  // https://stackoverflow.com/a/125106
+  const rect = element.getBoundingClientRect();
+  return (
+    rect.top >= 0 &&
+    rect.left >= 0 &&
+    rect.bottom <=
+      (window.innerHeight || document.documentElement.clientHeight) &&
+    rect.right <= (window.innerWidth || document.documentElement.clientWidth)
+  );
+}
+
 /**
  * Returns a callback that runs only when the document is visible.
  * - If the document is visible, runs immediately

src/utils/urlUtils.ts

@@ -46,7 +46,7 @@ export function selectAbsoluteUrl({
 }: {
   url?: string;
   baseURL?: string;
-}): string {
+} = {}): string {
   assertNotNullish(url, "selectAbsoluteUrl: The URL was not provided");
   if (canParseUrl(url)) {
     return url;