#4489 Report tab slice errors properly (#4623)

src/pageEditor/EditorContent.module.scss

@@ -0,0 +1,21 @@
+/*!
+ * Copyright (C) 2022 PixieBrix, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+.alertContainer {
+  margin: 0.5em;
+  max-width: 60vw;
+}

src/pageEditor/EditorContent.test.tsx

@@ -0,0 +1,117 @@
+/*
+ * Copyright (C) 2022 PixieBrix, Inc.
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Affero General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU Affero General Public License for more details.
+ *
+ * You should have received a copy of the GNU Affero General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+import { getCurrentURL } from "@/pageEditor/utils";
+import { formStateFactory } from "@/testUtils/factories";
+import { render, screen } from "@/pageEditor/testHelpers";
+import { actions as editorActions } from "@/pageEditor/slices/editorSlice";
+import { waitForEffect } from "@/testUtils/testHelpers";
+import React from "react";
+import EditorContent from "@/pageEditor/EditorContent";
+import { getInstalledExtensionPoints } from "@/contentScript/messenger/api";
+
+jest.mock("@/services/api", () => ({
+  appApi: {
+    endpoints: {
+      getMarketplaceListings: {
+        useQueryState: jest.fn(),
+      },
+    },
+  },
+  useGetMarketplaceTagsQuery: jest.fn(),
+  useGetMarketplaceListingsQuery: jest.fn(),
+  useGetEditablePackagesQuery: jest.fn(),
+  useGetRecipesQuery: jest.fn(),
+  useCreateRecipeMutation: jest.fn(),
+  useUpdateRecipeMutation: jest.fn(),
+}));
+jest.mock("@/components/asyncIcon", () => ({
+  useAsyncIcon: jest.fn(),
+}));
+jest.mock("@/telemetry/events", () => ({
+  reportEvent: jest.fn(),
+}));
+jest.mock("@/permissions", () => {
+  const permissions = {};
+  return {
+    extensionPermissions: jest.fn().mockResolvedValue(permissions),
+  };
+});
+jest.mock("@/background/messenger/api", () => ({
+  containsPermissions: jest.fn().mockResolvedValue(true),
+}));
+// Mock to support hook usage in the subtree, not relevant to UI tests here
+jest.mock("@/hooks/useRefresh");
+jest.mock("@/hooks/useTheme", () => ({
+  useGetTheme: jest.fn(),
+}));
+
+jest.mock("@/pageEditor/utils", () => {
+  const actual = jest.requireActual("@/pageEditor/utils");
+  return {
+    ...actual,
+    getCurrentURL: jest.fn(),
+  };
+});
+
+jest.mock("@/pageEditor/hooks/useCurrentUrl");
+
+jest.mock("@/contentScript/messenger/api", () => ({
+  getInstalledExtensionPoints: jest.fn(),
+}));
+
+describe("error alerting in the UI", () => {
+  test("shows error when checkAvailableDynamicElements fails", async () => {
+    const message = "testing error";
+    (getCurrentURL as jest.Mock).mockImplementation(() => {
+      throw new Error(message);
+    });
+
+    const formState = formStateFactory();
+    render(<EditorContent />, {
+      async setupRedux(dispatch) {
+        dispatch(editorActions.addElement(formState));
+        dispatch(editorActions.selectElement(formState.uuid));
+        await dispatch(editorActions.checkAvailableDynamicElements());
+      },
+    });
+
+    await waitForEffect();
+
+    expect(screen.getByText(message)).toBeInTheDocument();
+  });
+
+  test("shows error when checkAvailableInstalledExtensions fails", async () => {
+    const message = "testing error";
+    (getInstalledExtensionPoints as jest.Mock).mockImplementation(() => {
+      throw new Error(message);
+    });
+
+    const formState = formStateFactory();
+    render(<EditorContent />, {
+      async setupRedux(dispatch) {
+        dispatch(editorActions.addElement(formState));
+        dispatch(editorActions.selectElement(formState.uuid));
+        await dispatch(editorActions.checkAvailableInstalledExtensions());
+      },
+    });
+
+    await waitForEffect();
+
+    expect(screen.getByText(message)).toBeInTheDocument();
+  });
+});

src/pageEditor/EditorContent.tsx

@@ -38,10 +38,12 @@ import {
   selectTabHasPermissions,
   selectTabIsConnectingToContentScript,
 } from "@/pageEditor/tabState/tabStateSelectors";
-import useCurrentUrl from "./hooks/useCurrentUrl";
-import CantModifyPane from "./panes/CantModifyPane";
+import useCurrentUrl from "@/pageEditor/hooks/useCurrentUrl";
+import CantModifyPane from "@/pageEditor/panes/CantModifyPane";
 import { isScriptableUrl } from "@/utils/permissions";
 import { getErrorMessage } from "@/errors/errorHelpers";
+import Alert from "@/components/Alert";
+import styles from "./EditorContent.module.scss";
 
 const EditorContent: React.FC = () => {
   const tabHasPermissions = useSelector(selectTabHasPermissions);
@@ -96,6 +98,15 @@ const EditorContent: React.FC = () => {
     };
   }, [sessionId]);
 
+  // Always show the main error if present - keep this first
+  if (editorError) {
+    return (
+      <div className={styles.alertContainer}>
+        <Alert variant="danger">{getErrorMessage(editorError)}</Alert>
+      </div>
+    );
+  }
+
   if (!url) {
     // Don't show anything while it's loading the URL, nearly immediate
     return null;
@@ -115,14 +126,6 @@ const EditorContent: React.FC = () => {
     return <BetaPane />;
   }
 
-  if (editorError) {
-    return (
-      <div className="p-2">
-        <span className="text-danger">{getErrorMessage(editorError)}</span>
-      </div>
-    );
-  }
-
   if (activeElementId) {
     return <EditorPane />;
   }

src/pageEditor/slices/editorSlice.ts

@@ -758,6 +758,7 @@ export const editorSlice = createSlice({
         (state, { error }) => {
           state.isPendingInstalledExtensions = false;
           state.unavailableInstalledCount = 0;
+          state.error = error;
           reportError(error);
         }
       )
@@ -776,6 +777,7 @@ export const editorSlice = createSlice({
       .addCase(checkAvailableDynamicElements.rejected, (state, { error }) => {
         state.isPendingDynamicExtensions = false;
         state.unavailableDynamicCount = 0;
+        state.error = error;
         reportError(error);
       })
       .addCase(

src/pageEditor/tabState/tabStateSlice.ts

@@ -125,6 +125,7 @@ export const tabStateSlice = createSlice({
         state.isConnecting = false;
         state.frameState = defaultFrameState;
         state.error = serializeError(error);
+        reportError(error);
       })
       .addCase(awaitContextInvalidated.fulfilled, (state) => {
         state.isConnecting = false;
@@ -133,6 +134,7 @@ export const tabStateSlice = createSlice({
           "PixieBrix was updated or restarted. Reload the Page Editor to continue."
         );
         state.error = serializeError(error);
+        reportError(error);
       });
   },
 });

src/pageEditor/testHelpers.ts

@@ -28,6 +28,7 @@ import { logSlice } from "@/components/logViewer/logSlice";
 import { createRenderWithWrappers } from "@/testUtils/testHelpers";
 import analysisSlice from "@/analysis/analysisSlice";
 import pageEditorAnalysisManager from "./analysisManager";
+import { tabStateSlice } from "@/pageEditor/tabState/tabStateSlice";
 
 const renderWithWrappers = createRenderWithWrappers(() =>
   configureStore({
@@ -42,6 +43,7 @@ const renderWithWrappers = createRenderWithWrappers(() =>
       runtime: runtimeSlice.reducer,
       logs: logSlice.reducer,
       analysis: analysisSlice.reducer,
+      tabState: tabStateSlice.reducer,
       // This api reducer may be needed at some point, but it's not mocked properly yet, so
       //  we're not including it for now, until it becomes an issue.
       // [appApi.reducerPath]: appApi.reducer,

src/testUtils/testHelpers.tsx

@@ -33,9 +33,10 @@ import {
   PreloadedState,
   Reducer,
   ReducersMapObject,
+  ThunkDispatch,
 } from "@reduxjs/toolkit";
 import { Form, Formik, FormikValues } from "formik";
-import { Dispatch, Middleware } from "redux";
+import { Middleware } from "redux";
 import userEvent from "@testing-library/user-event";
 import { Expression, ExpressionType } from "@/core";
 import { noop } from "lodash";
@@ -127,7 +128,9 @@ export function createRenderFunctionWithRedux<
   };
 }
 
-type SetupRedux = (dispatch: Dispatch) => void;
+type SetupRedux = (
+  dispatch: ThunkDispatch<unknown, unknown, AnyAction>
+) => void;
 
 type WrapperOptions = Omit<RenderOptions, "wrapper"> & {
   initialValues?: FormikValues;