WD-8309 - feat: Display error when trying to retrieve models info (#1686

)

src/juju/api.test.ts

@@ -369,9 +369,9 @@ describe("Juju API", () => {
         () => state,
       );
       jest.advanceTimersByTime(LOGIN_TIMEOUT);
-      await expect(response).resolves.toBeNull();
+      await expect(response).rejects.toStrictEqual(new Error("timeout"));
       expect(console.error).toHaveBeenCalledWith(
-        "error connecting to model:",
+        "Error connecting to model:",
         "abc123",
         new Error("timeout"),
       );
@@ -456,15 +456,18 @@ describe("Juju API", () => {
       jest
         .spyOn(jujuLib, "connectAndLogin")
         .mockImplementation(async () => loginResponse);
-      const response = await fetchModelStatus(
+      const response = fetchModelStatus(
         "abc123",
         "wss://example.com/api",
         () => state,
       );
-      expect(response).toBeUndefined();
+      await expect(response).rejects.toStrictEqual(
+        new Error("Unable to fetch the status. Uh oh!"),
+      );
       expect(console.error).toHaveBeenCalledWith(
-        "Unable to fetch the status.",
-        "Uh oh!",
+        "Error connecting to model:",
+        "abc123",
+        new Error("Unable to fetch the status. Uh oh!"),
       );
       console.error = consoleError;
     });
@@ -535,12 +538,20 @@ describe("Juju API", () => {
         .spyOn(jujuLib, "connectAndLogin")
         .mockImplementation(async () => loginResponse);
       const dispatch = jest.fn();
-      await fetchAndStoreModelStatus(
+      const response = fetchAndStoreModelStatus(
         "abc123",
         "wss://example.com/api",
         dispatch,
         () => state,
       );
+      await expect(response).rejects.toStrictEqual(
+        new Error("Unable to fetch the status. "),
+      );
+      expect(console.error).toHaveBeenCalledWith(
+        "Error connecting to model:",
+        "abc123",
+        new Error("Unable to fetch the status. "),
+      );
       expect(dispatch).not.toHaveBeenCalled();
       console.error = consoleError;
     });
@@ -713,6 +724,97 @@ describe("Juju API", () => {
         new Error("Error while trying to dispatch!"),
       );
     });
+
+    it("should return a rejected promise when retrieving data for some models fails", async () => {
+      // The dispatch of updateModelInfo fails only for the first model.
+      // This would make 50% (1/2) of all models error out.
+      const dispatch = jest.fn().mockImplementationOnce(() => {
+        throw new Error("Error while dispatching updateModelInfo!");
+      });
+      const abc123 = modelInfoResultsFactory.build({
+        results: [
+          modelInfoResultFactory.build({
+            result: modelInfoFactory.build({
+              uuid: "abc123",
+            }),
+          }),
+        ],
+      });
+      const def456 = modelInfoResultsFactory.build({
+        results: [
+          modelInfoResultFactory.build({
+            result: modelInfoFactory.build({
+              uuid: "def456",
+            }),
+          }),
+        ],
+      });
+      const conn = {
+        facades: {
+          modelManager: {
+            modelInfo: jest
+              .fn()
+              .mockResolvedValueOnce(abc123)
+              .mockResolvedValueOnce(def456),
+          },
+        },
+      } as unknown as Connection;
+      const response = fetchAllModelStatuses(
+        "wss://example.com/api",
+        ["abc123", "def456"],
+        conn,
+        dispatch,
+        () => state,
+      );
+      await expect(response).rejects.toStrictEqual(
+        new Error("Unable to load some models."),
+      );
+    });
+
+    it("should return a rejected promise when retrieving data for all models", async () => {
+      // The dispatch of updateModelInfo fails for all models.
+      const dispatch = jest.fn().mockImplementation(() => {
+        throw new Error("Error while dispatching updateModelInfo!");
+      });
+      const abc123 = modelInfoResultsFactory.build({
+        results: [
+          modelInfoResultFactory.build({
+            result: modelInfoFactory.build({
+              uuid: "abc123",
+            }),
+          }),
+        ],
+      });
+      const def456 = modelInfoResultsFactory.build({
+        results: [
+          modelInfoResultFactory.build({
+            result: modelInfoFactory.build({
+              uuid: "def456",
+            }),
+          }),
+        ],
+      });
+      const conn = {
+        facades: {
+          modelManager: {
+            modelInfo: jest
+              .fn()
+              .mockResolvedValueOnce(abc123)
+              .mockResolvedValueOnce(def456),
+          },
+        },
+      } as unknown as Connection;
+      const response = fetchAllModelStatuses(
+        "wss://example.com/api",
+        ["abc123", "def456"],
+        conn,
+        dispatch,
+        () => state,
+      );
+      await expect(response).rejects.toStrictEqual(
+        new Error("Unable to load models."),
+      );
+    });
   });
 
   describe("fetchControllerList", () => {

src/juju/api.ts

@@ -42,6 +42,7 @@ import type { Credential } from "store/general/types";
 import { actions as jujuActions } from "store/juju";
 import { addControllerCloudRegion } from "store/juju/thunks";
 import type { Controller as JujuController } from "store/juju/types";
+import { ModelsError } from "store/middleware/model-poller";
 import type { RootState, Store } from "store/store";
 
 import { getModelByUUID } from "../store/juju/selectors";
@@ -260,8 +261,7 @@ export async function fetchModelStatus(
           // a location to notify the user. In the new watcher model that's
           // being implemented we will be able to surface this error in the
           // model details page.
-          console.error("Unable to fetch the status.", status);
-          return;
+          throw new Error(`Unable to fetch the status. ${status ?? ""}`);
         }
       }
 
@@ -284,7 +284,8 @@ export async function fetchModelStatus(
       }
       logout();
     } catch (error) {
-      console.error("error connecting to model:", modelUUID, error);
+      console.error("Error connecting to model:", modelUUID, error);
+      throw error;
     }
   }
   return status;
@@ -344,49 +345,64 @@ export async function fetchAllModelStatuses(
   getState: () => RootState,
 ) {
   const queue = new Limiter({ concurrency: 1 });
+  let modelErrorCount = 0;
   modelUUIDList.forEach((modelUUID) => {
     queue.push(async (done) => {
       if (isLoggedIn(getState(), wsControllerURL)) {
-        const modelWsControllerURL = getModelByUUID(
-          getState(),
-          modelUUID,
-        )?.wsControllerURL;
-        if (modelWsControllerURL) {
-          await fetchAndStoreModelStatus(
+        try {
+          const modelWsControllerURL = getModelByUUID(
+            getState(),
             modelUUID,
-            modelWsControllerURL,
-            dispatch,
-            getState,
-          );
-        }
-        const modelInfo = await fetchModelInfo(conn, modelUUID);
-        if (modelInfo) {
-          dispatch(
-            jujuActions.updateModelInfo({
-              modelInfo,
-              wsControllerURL,
-            }),
-          );
-        }
-        if (modelInfo?.results[0].result?.["is-controller"]) {
-          // If this is a controller model then update the
-          // controller data with this model data.
-          dispatch(
-            addControllerCloudRegion({ wsControllerURL, modelInfo }),
-          ).catch((error) =>
-            console.error(
-              "Error when trying to add controller cloud and region data.",
-              error,
-            ),
-          );
+          )?.wsControllerURL;
+          if (modelWsControllerURL) {
+            await fetchAndStoreModelStatus(
+              modelUUID,
+              modelWsControllerURL,
+              dispatch,
+              getState,
+            );
+          }
+          const modelInfo = await fetchModelInfo(conn, modelUUID);
+          if (modelInfo) {
+            dispatch(
+              jujuActions.updateModelInfo({
+                modelInfo,
+                wsControllerURL,
+              }),
+            );
+          }
+          if (modelInfo?.results[0].result?.["is-controller"]) {
+            // If this is a controller model then update the
+            // controller data with this model data.
+            dispatch(
+              addControllerCloudRegion({ wsControllerURL, modelInfo }),
+            ).catch((error) =>
+              console.error(
+                "Error when trying to add controller cloud and region data.",
+                error,
+              ),
+            );
+          }
+        } catch (error) {
+          modelErrorCount++;
         }
       }
       done();
     });
   });
-  return new Promise<void>((resolve) => {
+  return new Promise<void>((resolve, reject) => {
     queue.onDone(() => {
-      resolve();
+      // If errors appear in more than 10% of models, the promise should be
+      // rejected and the error further handled in modelPollerMiddleware().
+      modelErrorCount >= 0.1 * modelUUIDList.length
+        ? reject(
+            new Error(
+              modelErrorCount === modelUUIDList.length
+                ? ModelsError.LOAD_ALL_MODELS
+                : ModelsError.LOAD_SOME_MODELS,
+            ),
+          )
+        : resolve();
     });
   });
 }

src/pages/ModelsIndex/ModelsIndex.test.tsx

@@ -173,4 +173,26 @@ describe("Models Index page", () => {
     });
     expect(window.location.search).toBe(`?${params.toString()}`);
   });
+
+  it("should display the error notification", async () => {
+    state.juju.modelsError = "Oops!";
+    renderComponent(<ModelsIndex />, { state });
+    expect(screen.getByText(new RegExp(/Oops!/))).toBeInTheDocument();
+  });
+
+  it("should refresh the window when pressing the button in error notification", async () => {
+    const location = window.location;
+    Object.defineProperty(window, "location", {
+      value: { ...location, reload: jest.fn() },
+    });
+
+    state.juju.modelsError = "Oops!";
+    renderComponent(<ModelsIndex />, { state });
+    await userEvent.click(screen.getByRole("button", { name: "refreshing" }));
+    expect(window.location.reload).toHaveBeenCalled();
+
+    Object.defineProperty(window, "location", {
+      value: location,
+    });
+  });
 });

src/pages/ModelsIndex/ModelsIndex.tsx

@@ -1,4 +1,8 @@
-import { SearchAndFilter } from "@canonical/react-components";
+import {
+  Button,
+  Notification,
+  SearchAndFilter,
+} from "@canonical/react-components";
 import type { SearchAndFilterChip } from "@canonical/react-components/dist/components/SearchAndFilter/types";
 import type { ReactNode } from "react";
 import { useSelector } from "react-redux";
@@ -16,6 +20,7 @@ import {
   getGroupedModelStatusCounts,
   getModelData,
   getModelListLoaded,
+  getModelsError,
   hasModels,
 } from "store/juju/selectors";
 import { pluralize } from "store/juju/utils/models";
@@ -55,6 +60,7 @@ export default function Models() {
     custom: queryParams.custom,
   };
 
+  const modelsError = useAppSelector(getModelsError);
   const modelsLoaded = useAppSelector(getModelListLoaded);
   const hasSomeModels = useSelector(hasModels);
   // loop model data and pull out filter panel data
@@ -196,6 +202,15 @@ export default function Models() {
       titleComponent="div"
       titleClassName="u-no-max-width u-full-width"
     >
+      {modelsError ? (
+        <Notification severity="negative" title="Error">
+          {modelsError} Try{" "}
+          <Button appearance="link" onClick={() => window.location.reload()}>
+            refreshing
+          </Button>{" "}
+          the page.
+        </Notification>
+      ) : null}
       {content}
     </BaseLayout>
   );

src/store/juju/actions.test.ts

@@ -95,6 +95,21 @@ describe("actions", () => {
     });
   });
 
+  it("updateModelsError", () => {
+    expect(
+      actions.updateModelsError({
+        modelsError: null,
+        wsControllerURL: "wss://test.example.com",
+      }),
+    ).toStrictEqual({
+      type: "juju/updateModelsError",
+      payload: {
+        modelsError: null,
+        wsControllerURL: "wss://test.example.com",
+      },
+    });
+  });
+
   it("clearModelData", () => {
     expect(actions.clearModelData()).toStrictEqual({
       type: "juju/clearModelData",

src/store/juju/reducers.test.ts

@@ -160,6 +160,26 @@ describe("reducers", () => {
     });
   });
 
+  it("updateModelsError", () => {
+    const state = jujuStateFactory.build({
+      modelData: {
+        abc123: model,
+      },
+    });
+    expect(
+      reducer(
+        state,
+        actions.updateModelsError({
+          modelsError: null,
+          wsControllerURL: "wss://example.com",
+        }),
+      ),
+    ).toStrictEqual({
+      ...state,
+      modelsError: null,
+    });
+  });
+
   it("clearModelData", () => {
     const state = jujuStateFactory.build({
       modelData: {