WD-3060 - Handle login timeouts (#1457)

* Handle login timeouts.

src/components/App/App.test.tsx

@@ -1,19 +1,34 @@
-import { render } from "@testing-library/react";
+import { render, screen } from "@testing-library/react";
 import { Provider } from "react-redux";
 import * as reactRouterDOM from "react-router-dom";
 import configureStore from "redux-mock-store";
 
+import * as Routes from "components/Routes/Routes";
 import { rootStateFactory } from "testing/factories/root";
 
 import App from "./App";
 
-jest.mock("react-router-dom", () => ({
-  BrowserRouter: jest.fn(),
+jest.mock("components/Routes/Routes", () => ({
+  Routes: jest.fn(),
 }));
 
 const mockStore = configureStore([]);
 
 describe("App", () => {
+  let consoleError: Console["error"];
+
+  beforeEach(() => {
+    consoleError = console.error;
+    // Even though the error boundary catches the error, there is still a
+    // console.error in the test output.
+    console.error = jest.fn();
+  });
+
+  afterEach(() => {
+    console.error = consoleError;
+    jest.resetAllMocks();
+  });
+
   it("properly sets up Router", () => {
     const BrowserRouterSpy = jest
       .spyOn(reactRouterDOM, "BrowserRouter")
@@ -25,5 +40,32 @@ describe("App", () => {
       </Provider>
     );
     expect(BrowserRouterSpy.mock.calls[0][0].basename).toBe("/");
+    BrowserRouterSpy.mockRestore();
+  });
+
+  it("catches and displays errors", () => {
+    jest.spyOn(Routes, "Routes").mockImplementation(() => {
+      throw new Error("This is a thrown error");
+    });
+    const store = mockStore(rootStateFactory.withGeneralConfig().build());
+    render(
+      <Provider store={store}>
+        <App />
+      </Provider>
+    );
+    expect(screen.getByText("This is a thrown error")).toBeInTheDocument();
+  });
+
+  it("displays connection errors", () => {
+    const state = rootStateFactory
+      .withGeneralConfig()
+      .build({ general: { connectionError: "Can't connect" } });
+    const store = mockStore(state);
+    render(
+      <Provider store={store}>
+        <App />
+      </Provider>
+    );
+    expect(screen.getByText(/Can't connect/)).toBeInTheDocument();
   });
 });

src/components/App/App.tsx

@@ -2,6 +2,7 @@ import { initialize, pageview } from "react-ga";
 import { useSelector } from "react-redux";
 import { BrowserRouter as Router } from "react-router-dom";
 
+import ConnectionError from "components/ConnectionError";
 import ErrorBoundary from "components/ErrorBoundary/ErrorBoundary";
 import { Routes } from "components/Routes/Routes";
 import useLocalStorage from "hooks/useLocalStorage";
@@ -21,7 +22,9 @@ function App() {
   return (
     <Router basename={config?.baseAppURL}>
       <ErrorBoundary>
-        <Routes />
+        <ConnectionError>
+          <Routes />
+        </ConnectionError>
       </ErrorBoundary>
     </Router>
   );

src/components/ConnectionError/ConnectionError.test.tsx

@@ -0,0 +1,54 @@
+import { render, screen } from "@testing-library/react";
+import userEvent from "@testing-library/user-event";
+import { Provider } from "react-redux";
+import configureStore from "redux-mock-store";
+
+import { rootStateFactory } from "testing/factories/root";
+
+import ConnectionError from "./ConnectionError";
+
+const mockStore = configureStore();
+
+describe("ConnectionError", () => {
+  let reload: Location["reload"];
+
+  beforeEach(() => {
+    reload = window.location.reload;
+    Object.defineProperty(window, "location", {
+      value: { reload: jest.fn() },
+    });
+  });
+
+  afterEach(() => {
+    Object.defineProperty(window, "location", {
+      value: { reload },
+    });
+  });
+
+  it("displays connection errors", () => {
+    const state = rootStateFactory
+      .withGeneralConfig()
+      .build({ general: { connectionError: "Can't connect" } });
+    const store = mockStore(state);
+    render(
+      <Provider store={store}>
+        <ConnectionError />
+      </Provider>
+    );
+    expect(screen.getByText(/Can't connect/)).toBeInTheDocument();
+  });
+
+  it("can refresh the window", async () => {
+    const state = rootStateFactory
+      .withGeneralConfig()
+      .build({ general: { connectionError: "Can't connect" } });
+    const store = mockStore(state);
+    render(
+      <Provider store={store}>
+        <ConnectionError />
+      </Provider>
+    );
+    await userEvent.click(screen.getByRole("button", { name: "refreshing" }));
+    expect(window.location.reload).toHaveBeenCalled();
+  });
+});

src/components/ConnectionError/ConnectionError.tsx

@@ -0,0 +1,26 @@
+import { Button, Notification, Strip } from "@canonical/react-components";
+import type { PropsWithChildren } from "react";
+
+import { getConnectionError } from "store/general/selectors";
+import { useAppSelector } from "store/store";
+
+const ConnectionError = ({ children }: PropsWithChildren) => {
+  const error = useAppSelector(getConnectionError);
+  if (error) {
+    return (
+      <Strip>
+        <Notification severity="negative" title="Error">
+          {error} Try{" "}
+          <Button appearance="link" onClick={() => window.location.reload()}>
+            refreshing
+          </Button>{" "}
+          the page.
+        </Notification>
+      </Strip>
+    );
+  }
+
+  return <>{children}</>;
+};
+
+export default ConnectionError;

src/components/ConnectionError/index.ts

@@ -0,0 +1 @@
+export { default } from "./ConnectionError";

src/juju/api.test.ts

@@ -98,10 +98,9 @@ describe("Juju API", () => {
     });
 
     it("handles login errors", async () => {
-      const error = new Error("It didn't work!");
       const juju = {
         login: jest.fn().mockImplementation(() => {
-          throw error;
+          throw new Error();
         }),
       };
       jest.spyOn(jujuLib, "connect").mockImplementation(async () => juju);
@@ -113,7 +112,9 @@ describe("Juju API", () => {
         },
         false
       );
-      expect(response).toStrictEqual({ error });
+      expect(response).toStrictEqual({
+        error: "Could not log into controller",
+      });
     });
 
     it("starts pinging the connection", async () => {

src/juju/api.ts

@@ -30,6 +30,7 @@ import type { Dispatch } from "redux";
 import { isSet } from "components/utils";
 import bakery from "juju/bakery";
 import JIMMV2 from "juju/jimm-facade";
+import { actions as generalActions } from "store/general";
 import {
   getConfig,
   getControllerConnection,
@@ -118,7 +119,7 @@ function startPingerLoop(conn: ConnectionWithFacades) {
   const intervalId = window.setInterval(() => {
     conn.facades.pinger?.ping(null).catch((e: unknown) => {
       // If the pinger fails for whatever reason then cancel the ping.
-      console.error("pinger stopped,", e);
+      console.log("pinger stopped,", e);
       stopPingerLoop(intervalId);
     });
   }, PING_TIME);
@@ -159,11 +160,10 @@ export async function loginWithBakery(
   try {
     conn = await juju.login(loginParams, CLIENT_VERSION);
   } catch (error) {
-    return { error };
+    return { error: "Could not log into controller" };
   }
 
   const intervalId = conn ? startPingerLoop(conn) : null;
-
   return { conn, juju, intervalId };
 }
 
@@ -400,11 +400,19 @@ export async function fetchControllerList(
 ) {
   let controllers: JujuController[] | null = null;
   if (conn.facades.jimM) {
-    const response = await conn.facades.jimM?.listControllers();
-    controllers = response.controllers;
-    controllers?.forEach(
-      (c) => (c.additionalController = additionalController)
-    );
+    try {
+      const response = await conn.facades.jimM?.listControllers();
+      controllers = response.controllers;
+      controllers?.forEach(
+        (c) => (c.additionalController = additionalController)
+      );
+    } catch (error) {
+      dispatch(
+        generalActions.storeConnectionError(
+          "Unable to fetch the list of controllers."
+        )
+      );
+    }
   } else {
     // If we're not connected to a JIMM then call to get the controller config
     // and generate a fake controller list.

src/store/general/actions.test.ts

@@ -37,6 +37,13 @@ describe("actions", () => {
     });
   });
 
+  it("storeConnectionError", () => {
+    expect(actions.storeConnectionError("error")).toStrictEqual({
+      type: "general/storeConnectionError",
+      payload: "error",
+    });
+  });
+
   it("storeUserPass", () => {
     expect(
       actions.storeUserPass({

src/store/general/reducers.test.ts

@@ -56,6 +56,18 @@ describe("reducers", () => {
     });
   });
 
+  it("storeConnectionError", () => {
+    const state = generalStateFactory.build({
+      connectionError: "old error",
+    });
+    expect(
+      reducer(state, actions.storeConnectionError("new error"))
+    ).toStrictEqual({
+      ...state,
+      connectionError: "new error",
+    });
+  });
+
   it("storeUserPass", () => {
     const state = generalStateFactory.build();
     const credential = credentialFactory.build({

src/store/general/selectors.test.ts

@@ -18,6 +18,7 @@ import {
   isConnecting,
   isLoggedIn,
   getActiveUserControllerAccess,
+  getConnectionError,
 } from "./selectors";
 
 describe("selectors", () => {
@@ -50,6 +51,18 @@ describe("selectors", () => {
     ).toStrictEqual(credential);
   });
 
+  it("getConnectionError", () => {
+    expect(
+      getConnectionError(
+        rootStateFactory.build({
+          general: generalStateFactory.build({
+            connectionError: "error",
+          }),
+        })
+      )
+    ).toBe("error");
+  });
+
   it("getLoginError", () => {
     expect(
       getLoginError(

src/store/general/selectors.ts

@@ -27,6 +27,11 @@ export const getUserPass = createSelector(
   (sliceState, wsControllerURL) => sliceState?.credentials?.[wsControllerURL]
 );
 
+export const getConnectionError = createSelector(
+  [slice],
+  (sliceState) => sliceState?.connectionError
+);
+
 /**
   Fetches a login error from state
   @param state The application state.

src/store/general/slice.ts

@@ -1,4 +1,5 @@
 import { createSlice } from "@reduxjs/toolkit";
+import type { PayloadAction } from "@reduxjs/toolkit";
 
 import type { GeneralState } from "./types";
 
@@ -7,6 +8,7 @@ const slice = createSlice({
   initialState: {
     appVersion: null,
     config: null,
+    connectionError: null,
     controllerConnections: null,
     credentials: null,
     loginError: null,
@@ -25,6 +27,9 @@ const slice = createSlice({
     storeConfig: (state, action) => {
       state.config = action.payload;
     },
+    storeConnectionError: (state, action: PayloadAction<string>) => {
+      state.connectionError = action.payload;
+    },
     storeLoginError: (state, action) => {
       state.loginError = action.payload;
     },

src/store/general/types.ts

@@ -24,6 +24,7 @@ export type Credentials = Record<string, Credential>;
 export type GeneralState = {
   appVersion: string | null;
   config: Config | null;
+  connectionError?: string | null;
   controllerConnections: ControllerConnections | null;
   credentials: Credentials | null;
   loginError: string | null;

src/store/middleware/check-auth.ts

@@ -54,6 +54,7 @@ export const checkAuthMiddleware: Middleware<
       generalActions.logOut.type,
       generalActions.storeConfig.type,
       generalActions.storeLoginError.type,
+      generalActions.storeConnectionError.type,
       generalActions.storeUserPass.type,
       generalActions.storeVersion.type,
       generalActions.storeVisitURL.type,

src/testing/factories/general.ts

@@ -26,6 +26,7 @@ class GeneralStateFactory extends Factory<GeneralState> {
 export const generalStateFactory = GeneralStateFactory.define(() => ({
   appVersion: null,
   config: null,
+  connectionError: null,
   controllerConnections: null,
   credentials: null,
   loginError: null,