From aca9cdacf7bdc42ebc4844669c9bbbb1be8413fa Mon Sep 17 00:00:00 2001 From: Urvashi Sharma Date: Tue, 4 Feb 2025 19:53:35 +0530 Subject: [PATCH] feat(logging): Use loglevel library for error handling --- package.json | 1 + src/components/App/App.test.tsx | 17 ++++--- src/components/LogIn/LogIn.test.tsx | 5 --- .../LogIn/UserPassForm/UserPassForm.test.tsx | 20 ++++++--- .../LogIn/UserPassForm/UserPassForm.tsx | 3 +- src/components/ShareCard/ShareCard.test.tsx | 22 ++++++--- src/components/ShareCard/ShareCard.tsx | 3 +- src/components/UserMenu/UserMenu.test.tsx | 3 -- src/hooks/useLocalStorage.test.ts | 19 +++++--- src/hooks/useLocalStorage.ts | 5 ++- src/hooks/useLogout.test.tsx | 17 ++++--- src/hooks/useLogout.tsx | 3 +- src/index.test.tsx | 23 +++++----- src/index.tsx | 5 ++- src/juju/api.test.ts | 45 ++++++------------- src/juju/api.ts | 7 +-- .../Model/Logs/ActionLogs/ActionLogs.test.tsx | 17 ++++--- .../Model/Logs/ActionLogs/ActionLogs.tsx | 8 ++-- src/pages/ModelDetails/ModelDetails.test.tsx | 15 +++++-- src/pages/ModelDetails/ModelDetails.tsx | 3 +- src/pages/Permissions/Permissions.test.tsx | 4 -- src/panels/ActionsPanel/ActionsPanel.test.tsx | 15 +++++-- src/panels/ActionsPanel/ActionsPanel.tsx | 3 +- .../ConfirmationDialog.test.tsx | 18 ++++++-- .../ConfirmationDialog/ConfirmationDialog.tsx | 3 +- .../CharmsAndActionsPanel.test.tsx | 18 +++++--- .../CharmsAndActionsPanel.tsx | 3 +- src/panels/ConfigPanel/ConfigPanel.test.tsx | 17 ++++--- src/panels/ConfigPanel/ConfigPanel.tsx | 3 +- .../ConfirmationDialog.test.tsx | 23 ++++++---- .../ConfirmationDialog/ConfirmationDialog.tsx | 5 ++- src/store/app/thunks.test.ts | 19 +++++--- src/store/app/thunks.ts | 3 +- src/store/middleware/check-auth.test.ts | 13 ++++-- src/store/middleware/check-auth.ts | 5 ++- src/store/middleware/model-poller.test.ts | 34 +++++++------- src/store/middleware/model-poller.ts | 7 +-- src/store/store.ts | 3 +- yarn.lock | 3 +- 39 files changed, 260 insertions(+), 180 deletions(-) diff --git a/package.json b/package.json index c6704891a..8bcec67cd 100644 --- a/package.json +++ b/package.json @@ -65,6 +65,7 @@ "fuse.js": "7.0.0", "lodash.isequal": "4.5.0", "lodash.mergewith": "4.6.2", + "loglevel": "^1.9.2", "prism-react-renderer": "2.4.1", "prismjs": "1.29.0", "react": "18.3.1", diff --git a/src/components/App/App.test.tsx b/src/components/App/App.test.tsx index 39252e7a1..8794a1706 100644 --- a/src/components/App/App.test.tsx +++ b/src/components/App/App.test.tsx @@ -1,4 +1,5 @@ import { render, screen } from "@testing-library/react"; +import log from "loglevel"; import * as reactGA from "react-ga"; import { Provider } from "react-redux"; import configureStore from "redux-mock-store"; @@ -26,20 +27,22 @@ vi.mock("react-router", async () => { }; }); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.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 = vi.fn(); + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); }); afterEach(() => { - console.error = consoleError; vi.resetAllMocks(); vi.unstubAllEnvs(); }); diff --git a/src/components/LogIn/LogIn.test.tsx b/src/components/LogIn/LogIn.test.tsx index b4c012b42..d5593d7da 100644 --- a/src/components/LogIn/LogIn.test.tsx +++ b/src/components/LogIn/LogIn.test.tsx @@ -166,9 +166,6 @@ describe("LogIn", () => { }); it("should remove the authentication request when clicking the authenticate button", async () => { - const consoleError = console.error; - console.error = vi.fn(); - const state = rootStateFactory.build({ general: generalStateFactory.withConfig().build({ config: configFactory.build({ @@ -186,7 +183,5 @@ describe("LogIn", () => { { pointerEventsCheck: 0 }, ); expect(screen.queryByTestId("toast-card")).not.toBeInTheDocument(); - - console.error = consoleError; }); }); diff --git a/src/components/LogIn/UserPassForm/UserPassForm.test.tsx b/src/components/LogIn/UserPassForm/UserPassForm.test.tsx index cf5419c9b..d816db4dd 100644 --- a/src/components/LogIn/UserPassForm/UserPassForm.test.tsx +++ b/src/components/LogIn/UserPassForm/UserPassForm.test.tsx @@ -1,5 +1,6 @@ import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import { vi } from "vitest"; import { thunks as appThunks } from "store/app"; @@ -13,7 +14,19 @@ import { Label } from "../types"; import UserPassForm from "./UserPassForm"; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("UserPassForm", () => { + beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); + }); + afterEach(() => { vi.restoreAllMocks(); }); @@ -59,9 +72,6 @@ describe("UserPassForm", () => { }); it("should display console error when trying to log in", async () => { - const consoleError = console.error; - console.error = vi.fn(); - vi.spyOn(appThunks, "connectAndStartPolling").mockImplementation( vi.fn().mockReturnValue({ type: "connectAndStartPolling" }), ); @@ -82,11 +92,9 @@ describe("UserPassForm", () => { renderComponent(); await userEvent.click(screen.getByRole("button")); expect(appThunks.connectAndStartPolling).toHaveBeenCalledTimes(1); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( Label.POLLING_ERROR, new Error("Error while dispatching connectAndStartPolling!"), ); - - console.error = consoleError; }); }); diff --git a/src/components/LogIn/UserPassForm/UserPassForm.tsx b/src/components/LogIn/UserPassForm/UserPassForm.tsx index 1a631848e..04cbf36d5 100644 --- a/src/components/LogIn/UserPassForm/UserPassForm.tsx +++ b/src/components/LogIn/UserPassForm/UserPassForm.tsx @@ -1,4 +1,5 @@ import { unwrapResult } from "@reduxjs/toolkit"; +import log from "loglevel"; import type { FormEvent } from "react"; import bakery from "juju/bakery"; @@ -35,7 +36,7 @@ const UserPassForm = () => { if (bakery) { dispatch(appThunks.connectAndStartPolling()) .then(unwrapResult) - .catch((error) => console.error(Label.POLLING_ERROR, error)); + .catch((error) => log.error(Label.POLLING_ERROR, error)); } } diff --git a/src/components/ShareCard/ShareCard.test.tsx b/src/components/ShareCard/ShareCard.test.tsx index 3b6c2e3d6..a6271dd89 100644 --- a/src/components/ShareCard/ShareCard.test.tsx +++ b/src/components/ShareCard/ShareCard.test.tsx @@ -1,11 +1,24 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import { vi } from "vitest"; import ShareCard from "./ShareCard"; import { Label } from "./types"; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("Share Card", () => { + beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); + }); + it("should display appropriate text", () => { render( { expect(accessSelectChangeFn).toHaveBeenCalled(); }); - it("should display console error when trying to change access", async () => { - const consoleError = console.error; - console.error = vi.fn(); - + it("should log error when trying to change access", async () => { const removeUserFn = vi.fn(); const accessSelectChangeFn = vi.fn(() => Promise.reject(new Error())); render( @@ -91,11 +101,9 @@ describe("Share Card", () => { />, ); await userEvent.selectOptions(screen.getByRole("combobox"), "write"); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( Label.ACCESS_CHANGE_ERROR, new Error(), ); - - console.error = consoleError; }); }); diff --git a/src/components/ShareCard/ShareCard.tsx b/src/components/ShareCard/ShareCard.tsx index 6f98b9901..c18de08a7 100644 --- a/src/components/ShareCard/ShareCard.tsx +++ b/src/components/ShareCard/ShareCard.tsx @@ -1,5 +1,6 @@ import type { ErrorResults } from "@canonical/jujulib/dist/api/facades/model-manager/ModelManagerV9"; import { Button, Select } from "@canonical/react-components"; +import log from "loglevel"; import { useEffect, useState } from "react"; import SlideDownFadeOut from "animations/SlideDownFadeOut"; @@ -117,7 +118,7 @@ export default function ShareCard({ return; }) .catch((error) => - console.error(Label.ACCESS_CHANGE_ERROR, error), + log.error(Label.ACCESS_CHANGE_ERROR, error), ); }} value={access} diff --git a/src/components/UserMenu/UserMenu.test.tsx b/src/components/UserMenu/UserMenu.test.tsx index 5f4205850..ce8ea5d87 100644 --- a/src/components/UserMenu/UserMenu.test.tsx +++ b/src/components/UserMenu/UserMenu.test.tsx @@ -13,7 +13,6 @@ import UserMenu from "./UserMenu"; describe("User Menu", () => { let state: RootState; - const consoleError = console.error; beforeEach(() => { state = rootStateFactory.build({ @@ -33,12 +32,10 @@ describe("User Menu", () => { }, }), }); - console.error = vi.fn(); }); afterEach(() => { vi.restoreAllMocks(); - console.error = consoleError; }); it("is inactive by default", () => { diff --git a/src/hooks/useLocalStorage.test.ts b/src/hooks/useLocalStorage.test.ts index f7fd8c7ac..def992eec 100644 --- a/src/hooks/useLocalStorage.test.ts +++ b/src/hooks/useLocalStorage.test.ts @@ -1,18 +1,23 @@ import { act, renderHook } from "@testing-library/react"; +import log from "loglevel"; import { vi } from "vitest"; import useLocalStorage from "./useLocalStorage"; -describe("useLocalStorage", () => { - let consoleError: Console["error"]; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); +describe("useLocalStorage", () => { beforeEach(() => { - consoleError = console.error; - console.error = vi.fn(); + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); }); afterEach(() => { - console.error = consoleError; localStorage.clear(); }); @@ -38,7 +43,7 @@ describe("useLocalStorage", () => { const { result } = renderHook(() => useLocalStorage("test-key", "init-val"), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Unable to parse local storage:", expect.any(Error), ); @@ -71,7 +76,7 @@ describe("useLocalStorage", () => { act(() => { setValue(circular as unknown as string); }); - expect(console.error).toHaveBeenCalledWith(expect.any(Error)); + expect(log.error).toHaveBeenCalledWith(expect.any(Error)); const [value] = result.current; expect(value).toBe("init-val"); expect(JSON.parse(localStorage.getItem("test-key") ?? "")).toBe("init-val"); diff --git a/src/hooks/useLocalStorage.ts b/src/hooks/useLocalStorage.ts index 6b365112a..d75e78579 100644 --- a/src/hooks/useLocalStorage.ts +++ b/src/hooks/useLocalStorage.ts @@ -1,3 +1,4 @@ +import log from "loglevel"; import { useState } from "react"; function useLocalStorage( @@ -12,7 +13,7 @@ function useLocalStorage( return item ? JSON.parse(item) : initialValue; } catch (error) { // Not shown in UI. Logged for debugging purposes. - console.error("Unable to parse local storage:", error); + log.error("Unable to parse local storage:", error); return initialValue; } }); @@ -26,7 +27,7 @@ function useLocalStorage( window.localStorage.setItem(key, stringified); } catch (error) { // Not shown in UI. Logged for debugging purposes. - console.error(error); + log.error(error); } }; diff --git a/src/hooks/useLogout.test.tsx b/src/hooks/useLogout.test.tsx index a8f58fc96..9e607d11a 100644 --- a/src/hooks/useLogout.test.tsx +++ b/src/hooks/useLogout.test.tsx @@ -1,5 +1,6 @@ import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import { vi } from "vitest"; import { Label } from "hooks/useLogout"; @@ -9,15 +10,17 @@ import { renderComponent, renderWrappedHook } from "testing/utils"; import useLogout from "./useLogout"; -describe("useLogout", () => { - const consoleError = console.error; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); +describe("useLogout", () => { beforeEach(() => { - console.error = vi.fn(); - }); - - afterEach(() => { - console.error = consoleError; + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); }); it("should logout", async () => { diff --git a/src/hooks/useLogout.tsx b/src/hooks/useLogout.tsx index e3fd01d8c..ffa5effcc 100644 --- a/src/hooks/useLogout.tsx +++ b/src/hooks/useLogout.tsx @@ -1,5 +1,6 @@ import { Button } from "@canonical/react-components"; import { unwrapResult } from "@reduxjs/toolkit"; +import log from "loglevel"; import reactHotToast from "react-hot-toast"; import ToastCard from "components/ToastCard"; @@ -30,7 +31,7 @@ const useLogout = () => { )); - console.error(Label.LOGOUT_ERROR, error); + log.error(Label.LOGOUT_ERROR, error); }); }; }; diff --git a/src/index.test.tsx b/src/index.test.tsx index 603333b63..a86b263d8 100644 --- a/src/index.test.tsx +++ b/src/index.test.tsx @@ -1,4 +1,5 @@ import { screen, waitFor } from "@testing-library/dom"; +import log from "loglevel"; import type { UnknownAction } from "redux"; import { vi } from "vitest"; @@ -33,11 +34,18 @@ vi.mock("store", () => { }; }); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + const appVersion = packageJSON.version; describe("renderApp", () => { let rootNode: HTMLElement; - const consoleError = console.error; const windowLocation = window.location; beforeEach(() => { @@ -47,7 +55,7 @@ describe("renderApp", () => { value: new URL(window.location.href), }); // Hide the config errors from the test output. - console.error = vi.fn(); + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); rootNode = document.createElement("div"); rootNode.setAttribute("id", ROOT_ID); document.body.appendChild(rootNode); @@ -59,7 +67,6 @@ describe("renderApp", () => { enumerable: true, value: windowLocation, }); - console.error = consoleError; if (rootNode) { document.body.removeChild(rootNode); } @@ -250,14 +257,8 @@ describe("renderApp", () => { }); describe("getControllerAPIEndpointErrors", () => { - const consoleError = console.error; - beforeEach(() => { - console.error = vi.fn(); - }); - - afterEach(() => { - console.error = consoleError; + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); }); it("should handle secure protocol", () => { @@ -329,7 +330,7 @@ describe("getControllerAPIEndpointErrors", () => { renderApp(); expect(appThunks.connectAndStartPolling).toHaveBeenCalledTimes(1); await waitFor(() => - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( Label.POLLING_ERROR, new Error("Error while dispatching connectAndStartPolling!"), ), diff --git a/src/index.tsx b/src/index.tsx index 24709107a..c29c5f275 100644 --- a/src/index.tsx +++ b/src/index.tsx @@ -1,6 +1,7 @@ import { Notification, Strip } from "@canonical/react-components"; import { unwrapResult } from "@reduxjs/toolkit"; import * as Sentry from "@sentry/browser"; +import log from "loglevel"; import { StrictMode } from "react"; import type { Root } from "react-dom/client"; import { createRoot } from "react-dom/client"; @@ -124,7 +125,7 @@ function bootstrap() { , ); - console.error(error); + log.error(error); return; } // It's possible that the charm is generating a relative path for the @@ -159,7 +160,7 @@ function bootstrap() { reduxStore .dispatch(appThunks.connectAndStartPolling()) .then(unwrapResult) - .catch((error) => console.error(Label.POLLING_ERROR, error)); + .catch((error) => log.error(Label.POLLING_ERROR, error)); } getRoot()?.render( diff --git a/src/juju/api.test.ts b/src/juju/api.test.ts index f7e85beaf..5ce448447 100644 --- a/src/juju/api.test.ts +++ b/src/juju/api.test.ts @@ -2,6 +2,7 @@ import type { Client, Connection } from "@canonical/jujulib"; import * as jujuLib from "@canonical/jujulib"; import * as jujuLibVersions from "@canonical/jujulib/dist/api/versions"; import { waitFor } from "@testing-library/react"; +import log from "loglevel"; import { vi } from "vitest"; import { actions as generalActions } from "store/general"; @@ -67,8 +68,17 @@ vi.mock("@canonical/jujulib/dist/api/versions", () => ({ jujuUpdateAvailable: vi.fn(), })); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("Juju API", () => { beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); vi.useFakeTimers(); }); @@ -213,8 +223,6 @@ describe("Juju API", () => { it("stops pinging the connection if there is an error", async () => { const clearInterval = vi.spyOn(window, "clearInterval"); - const consoleError = console.error; - console.error = vi.fn(); const ping = vi.fn().mockRejectedValue("Failed"); const conn = { facades: { @@ -241,21 +249,10 @@ describe("Juju API", () => { expect(clearInterval).toHaveBeenCalled(); }); clearInterval.mockRestore(); - console.error = consoleError; }); }); describe("connectAndLoginWithTimeout", () => { - const consoleError = console.error; - - beforeEach(() => { - console.error = vi.fn(); - }); - - afterEach(() => { - console.error = consoleError; - }); - it("can connect and log in", async () => { const juju = { logout: vi.fn(), @@ -367,8 +364,6 @@ describe("Juju API", () => { }); it("handles timeout errors", async () => { - const consoleError = console.error; - console.error = vi.fn(); vi.spyOn(jujuLib, "connectAndLogin").mockImplementation( async () => new Promise((resolve) => { @@ -384,12 +379,11 @@ describe("Juju API", () => { await expect(response).rejects.toStrictEqual( new Error(Label.LOGIN_TIMEOUT_ERROR), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Error connecting to model:", "abc123", new Error(Label.LOGIN_TIMEOUT_ERROR), ); - console.error = consoleError; }); it("can fetch the status", async () => { @@ -539,8 +533,6 @@ describe("Juju API", () => { }); it("handles status error response", async () => { - const consoleError = console.error; - console.error = vi.fn(); const loginResponse = { conn: { facades: { @@ -562,12 +554,11 @@ describe("Juju API", () => { await expect(response).rejects.toStrictEqual( new Error("Unable to fetch the status. Uh oh!"), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Error connecting to model:", "abc123", new Error("Unable to fetch the status. Uh oh!"), ); - console.error = consoleError; }); }); @@ -677,8 +668,6 @@ describe("Juju API", () => { }); it("handles no model status returned", async () => { - const consoleError = console.error; - console.error = vi.fn(); const loginResponse = { conn: { facades: { @@ -702,22 +691,19 @@ describe("Juju API", () => { await expect(response).rejects.toStrictEqual( new Error("Unable to fetch the status. Status not returned."), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Error connecting to model:", "abc123", new Error("Unable to fetch the status. Status not returned."), ); expect(dispatch).not.toHaveBeenCalled(); - console.error = consoleError; }); }); describe("fetchAllModelStatuses", () => { let state: RootState; - const consoleError = console.error; beforeEach(() => { - console.error = vi.fn(); // Need to use real timers so the promises resolve in the tests. vi.useRealTimers(); state = rootStateFactory.build({ @@ -749,7 +735,6 @@ describe("Juju API", () => { }); afterEach(() => { - console.error = consoleError; vi.restoreAllMocks(); }); @@ -865,7 +850,6 @@ describe("Juju API", () => { }); it("should console error when trying to update controller cloud and region", async () => { - const errorSpy = vi.spyOn(console, "error"); const dispatch = vi .fn() .mockReturnValueOnce(null) @@ -896,7 +880,7 @@ describe("Juju API", () => { ); expect(dispatch).toHaveBeenCalledTimes(2); await waitFor(() => - expect(errorSpy).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Error when trying to add controller cloud and region data.", new Error("Error while trying to dispatch!"), ), @@ -1233,7 +1217,6 @@ describe("Juju API", () => { it("should handle error when unable to fetch the list of controllers", async () => { const dispatch = vi.fn(); - console.error = vi.fn(); const conn = { facades: { jimM: { diff --git a/src/juju/api.ts b/src/juju/api.ts index 82f81caed..5ce66fe9d 100644 --- a/src/juju/api.ts +++ b/src/juju/api.ts @@ -17,6 +17,7 @@ import { jujuUpdateAvailable } from "@canonical/jujulib/dist/api/versions"; import type { ValueOf } from "@canonical/react-components"; import { unwrapResult } from "@reduxjs/toolkit"; import Limiter from "async-limiter"; +import log from "loglevel"; import type { Dispatch } from "redux"; import bakery from "juju/bakery"; @@ -123,7 +124,7 @@ function startPingerLoop(conn: ConnectionWithFacades) { conn.facades.pinger?.ping(null).catch((e) => { // If the pinger fails for whatever reason then cancel the ping. // Not shown in UI. Logged for debugging purposes. - console.error("pinger stopped,", e); + log.error("pinger stopped,", e); stopPingerLoop(intervalId); }); }, PING_TIME); @@ -270,7 +271,7 @@ export async function fetchModelStatus( } logout(); } catch (error) { - console.error("Error connecting to model:", modelUUID, error); + log.error("Error connecting to model:", modelUUID, error); throw error; } } @@ -389,7 +390,7 @@ export async function fetchAllModelStatuses( .then(unwrapResult) .catch((error) => // Not shown in UI. Logged for debugging purposes. - console.error( + log.error( "Error when trying to add controller cloud and region data.", error, ), diff --git a/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.test.tsx b/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.test.tsx index 5072c8dbb..b3cd96fa0 100644 --- a/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.test.tsx +++ b/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.test.tsx @@ -1,6 +1,7 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { add } from "date-fns"; +import log from "loglevel"; import { vi } from "vitest"; import * as actionsHooks from "juju/api-hooks/actions"; @@ -102,12 +103,21 @@ vi.mock("juju/api-hooks/actions", () => { }; }); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("Action Logs", () => { let state: RootState; const path = "/models/:userName/:modelName"; const url = "/models/eggman@external/group-test?activeView=action-logs"; beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); vi.spyOn(actionsHooks, "useQueryOperationsList").mockImplementation(() => vi.fn().mockImplementation(() => Promise.resolve(mockOperationResults)), ); @@ -356,9 +366,6 @@ describe("Action Logs", () => { }); it("should show error when fetching action logs and refetch action logs", async () => { - const consoleError = console.error; - console.error = vi.fn(); - const queryOperationsListSpy = vi .fn() .mockImplementation(() => @@ -370,7 +377,7 @@ describe("Action Logs", () => { renderComponent(, { path, url, state }); expect(queryOperationsListSpy).toHaveBeenCalledTimes(1); await waitFor(() => { - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( Label.FETCH_ERROR, new Error("Error while querying operations list."), ); @@ -384,7 +391,5 @@ describe("Action Logs", () => { expect(refetchButton).toHaveTextContent("refetch"); await userEvent.click(refetchButton); expect(queryOperationsListSpy).toHaveBeenCalledTimes(2); - - console.error = consoleError; }); }); diff --git a/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.tsx b/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.tsx index aa25aa00d..c4cddd1a6 100644 --- a/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.tsx +++ b/src/pages/EntityDetails/Model/Logs/ActionLogs/ActionLogs.tsx @@ -8,6 +8,7 @@ import { Icon, ModularTable, } from "@canonical/react-components"; +import log from "loglevel"; import type { ReactNode } from "react"; import { useCallback, useEffect, useMemo, useState } from "react"; import { useSelector } from "react-redux"; @@ -113,10 +114,7 @@ const generateApplicationRow = ( const appName = parts && parts[1]; if (!appName) { // Not shown in UI. Logged for debugging purposes. - console.error( - "Unable to parse action receiver", - actionData.action?.receiver, - ); + log.error("Unable to parse action receiver", actionData.action?.receiver); return null; } return { @@ -195,7 +193,7 @@ export default function ActionLogs() { setInlineErrors(InlineErrors.FETCH, null); } catch (error) { setInlineErrors(InlineErrors.FETCH, Label.FETCH_ERROR); - console.error(Label.FETCH_ERROR, error); + log.error(Label.FETCH_ERROR, error); } finally { setFetchedOperations(true); } diff --git a/src/pages/ModelDetails/ModelDetails.test.tsx b/src/pages/ModelDetails/ModelDetails.test.tsx index fb15ca8fa..f748bb0c8 100644 --- a/src/pages/ModelDetails/ModelDetails.test.tsx +++ b/src/pages/ModelDetails/ModelDetails.test.tsx @@ -1,6 +1,7 @@ import type { Connection } from "@canonical/jujulib"; import * as jujuLib from "@canonical/jujulib"; import { screen, waitFor } from "@testing-library/react"; +import log from "loglevel"; import { vi } from "vitest"; import * as juju from "juju/api"; @@ -37,6 +38,14 @@ vi.mock("@canonical/jujulib", () => ({ connectAndLogin: vi.fn(), })); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("ModelDetails", () => { let state: RootState; let client: { @@ -50,6 +59,7 @@ describe("ModelDetails", () => { }); beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); state = rootStateFactory.withGeneralConfig().build({ juju: jujuStateFactory.build({ models: { @@ -213,8 +223,6 @@ describe("ModelDetails", () => { }); it("should display console error when trying to stop model watcher", async () => { - const consoleError = console.error; - console.error = vi.fn(); vi.spyOn(juju, "startModelWatcher").mockImplementation( vi.fn().mockResolvedValue({ conn: client.conn, @@ -234,10 +242,9 @@ describe("ModelDetails", () => { ); unmount(); await waitFor(() => expect(juju.stopModelWatcher).toHaveBeenCalledTimes(1)); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ModelDetailsLabel.MODEL_WATCHER_ERROR, new Error("Failed to stop model watcher!"), ); - console.error = consoleError; }); }); diff --git a/src/pages/ModelDetails/ModelDetails.tsx b/src/pages/ModelDetails/ModelDetails.tsx index 978ff2db2..f971eb06e 100644 --- a/src/pages/ModelDetails/ModelDetails.tsx +++ b/src/pages/ModelDetails/ModelDetails.tsx @@ -1,4 +1,5 @@ import type { AllWatcherId } from "@canonical/jujulib/dist/api/facades/client/ClientV6"; +import log from "loglevel"; import { useEffect, useState } from "react"; import { useDispatch, useSelector } from "react-redux"; import { Route, Routes, useParams } from "react-router"; @@ -70,7 +71,7 @@ export default function ModelDetails() { ).catch((error) => // Error doesn’t interfere with the user’s interaction with the // dashboard. Not shown in UI. Logged for debugging purposes. - console.error(Label.MODEL_WATCHER_ERROR, error), + log.error(Label.MODEL_WATCHER_ERROR, error), ); } }; diff --git a/src/pages/Permissions/Permissions.test.tsx b/src/pages/Permissions/Permissions.test.tsx index 1292ad0ec..2b079ad2f 100644 --- a/src/pages/Permissions/Permissions.test.tsx +++ b/src/pages/Permissions/Permissions.test.tsx @@ -12,10 +12,7 @@ import Permissions from "./Permissions"; const mock = new MockAdapter(axiosInstance); describe("Permissions", () => { - const consoleError = console.error; - beforeEach(() => { - console.error = vi.fn(); mock.reset(); mock.onGet(endpoints().whoami).reply(200, { data: { @@ -29,7 +26,6 @@ describe("Permissions", () => { }); afterEach(() => { - console.error = consoleError; vi.restoreAllMocks(); }); diff --git a/src/panels/ActionsPanel/ActionsPanel.test.tsx b/src/panels/ActionsPanel/ActionsPanel.test.tsx index d004a9971..f71c0a0d8 100644 --- a/src/panels/ActionsPanel/ActionsPanel.test.tsx +++ b/src/panels/ActionsPanel/ActionsPanel.test.tsx @@ -1,5 +1,6 @@ import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import { vi } from "vitest"; import * as actionsHooks from "juju/api-hooks/actions"; @@ -60,15 +61,22 @@ vi.mock("juju/api-hooks/actions", () => { }; }); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("ActionsPanel", () => { - const consoleError = console.error; let state: RootState; const path = "/models/:userName/:modelName/app/:appName"; const url = "/models/user-eggman@external/group-test/app/kubernetes-master?panel=execute-action&units=ceph%2F0,ceph%2F1"; beforeEach(() => { - console.error = vi.fn(); + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); const getActionsForApplicationSpy = vi .fn() .mockImplementation(() => Promise.resolve(mockResponse)); @@ -89,7 +97,6 @@ describe("ActionsPanel", () => { }); afterEach(() => { - console.error = consoleError; vi.resetModules(); vi.restoreAllMocks(); }); @@ -328,7 +335,7 @@ describe("ActionsPanel", () => { await waitFor(() => expect(getActionsForApplicationSpy).toHaveBeenCalledTimes(1), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ActionsPanelLabel.GET_ACTIONS_ERROR, new Error("Error while trying to get actions for application!"), ); diff --git a/src/panels/ActionsPanel/ActionsPanel.tsx b/src/panels/ActionsPanel/ActionsPanel.tsx index 7bf35e5b3..537996682 100644 --- a/src/panels/ActionsPanel/ActionsPanel.tsx +++ b/src/panels/ActionsPanel/ActionsPanel.tsx @@ -1,4 +1,5 @@ import { ActionButton, Button } from "@canonical/react-components"; +import log from "loglevel"; import { useCallback, useEffect, useMemo, useRef, useState } from "react"; import { useSelector } from "react-redux"; import { useParams } from "react-router"; @@ -100,7 +101,7 @@ export default function ActionsPanel(): JSX.Element { }) .catch((error) => { setInlineErrors(InlineErrors.GET_ACTION, Label.GET_ACTIONS_ERROR); - console.error(Label.GET_ACTIONS_ERROR, error); + log.error(Label.GET_ACTIONS_ERROR, error); }) .finally(() => { setFetchingActionData(false); diff --git a/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.test.tsx b/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.test.tsx index 8c6991ebe..4a3858de0 100644 --- a/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.test.tsx +++ b/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.test.tsx @@ -1,5 +1,6 @@ import { screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import { vi } from "vitest"; import * as actionsHooks from "juju/api-hooks/actions"; @@ -11,11 +12,23 @@ import { InlineErrors } from "../types"; import ConfirmationDialog from "./ConfirmationDialog"; import { Label } from "./types"; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("ConfirmationDialog", () => { const path = "/models/:userName/:modelName/app/:appName"; const url = "/models/user-eggman@external/group-test/app/kubernetes-master?panel=execute-action&units=ceph%2F0,ceph%2F1"; + beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); + }); + afterEach(() => { vi.resetModules(); vi.restoreAllMocks(); @@ -106,8 +119,6 @@ describe("ConfirmationDialog", () => { }); it("should set inline error if execute action fails", async () => { - const consoleError = console.error; - console.error = vi.fn(); const mockSetInlineErrors = vi.fn(); const executeActionOnUnitsSpy = vi .fn() @@ -135,10 +146,9 @@ describe("ConfirmationDialog", () => { InlineErrors.EXECUTE_ACTION, Label.EXECUTE_ACTION_ERROR, ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( Label.EXECUTE_ACTION_ERROR, new Error("mock error"), ); - console.error = consoleError; }); }); diff --git a/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.tsx b/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.tsx index 71b554498..d43632525 100644 --- a/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.tsx +++ b/src/panels/ActionsPanel/ConfirmationDialog/ConfirmationDialog.tsx @@ -1,4 +1,5 @@ import { ConfirmationModal } from "@canonical/react-components"; +import log from "loglevel"; import { useParams } from "react-router"; import usePortal from "react-useportal"; @@ -67,7 +68,7 @@ const ConfirmationDialog = ({ Label.EXECUTE_ACTION_ERROR, ); setIsExecutingAction(false); - console.error(Label.EXECUTE_ACTION_ERROR, error); + log.error(Label.EXECUTE_ACTION_ERROR, error); }); }} close={() => setConfirmType(null)} diff --git a/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.test.tsx b/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.test.tsx index d5127909e..90bab568c 100644 --- a/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.test.tsx +++ b/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.test.tsx @@ -1,5 +1,6 @@ import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import { vi } from "vitest"; import * as juju from "juju/api"; @@ -25,8 +26,15 @@ import { CharmsPanelLabel } from "../CharmsPanel"; import CharmsAndActionsPanel from "./CharmsAndActionsPanel"; import { Label as CharmsAndActionsPanelLabel } from "./types"; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("CharmsAndActionsPanel", () => { - const consoleError = console.error; let state: RootState; const path = urls.model.index(null); const url = urls.model.index({ @@ -35,7 +43,7 @@ describe("CharmsAndActionsPanel", () => { }); beforeEach(() => { - console.error = vi.fn(); + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); vi.resetAllMocks(); state = rootStateFactory.build({ @@ -60,10 +68,6 @@ describe("CharmsAndActionsPanel", () => { }); }); - afterEach(() => { - console.error = consoleError; - }); - it("should display the spinner before loading the panel", async () => { vi.spyOn(juju, "getCharmsURLFromApplications").mockImplementation(() => Promise.resolve([]), @@ -163,7 +167,7 @@ describe("CharmsAndActionsPanel", () => { } = renderComponent(, { path, url, state }); expect(juju.getCharmsURLFromApplications).toHaveBeenCalledTimes(1); await waitFor(() => - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( CharmsAndActionsPanelLabel.GET_URL_ERROR, new Error("Error while calling getCharmsURLFromApplications"), ), diff --git a/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.tsx b/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.tsx index 02d14b999..747015c92 100644 --- a/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.tsx +++ b/src/panels/CharmsAndActionsPanel/CharmsAndActionsPanel.tsx @@ -1,4 +1,5 @@ import { Button } from "@canonical/react-components"; +import log from "loglevel"; import { useCallback, useEffect, useState } from "react"; import { useDispatch, useSelector } from "react-redux"; import { useParams } from "react-router"; @@ -76,7 +77,7 @@ const CharmsAndActionsPanel = () => { }) .catch((error) => { setInlineErrors(InlineErrors.GET_URL, Label.GET_URL_ERROR); - console.error(Label.GET_URL_ERROR, error); + log.error(Label.GET_URL_ERROR, error); }); }, [dispatch, getState, modelUUID, selectedApplications, setInlineErrors]); diff --git a/src/panels/ConfigPanel/ConfigPanel.test.tsx b/src/panels/ConfigPanel/ConfigPanel.test.tsx index 46df39fc9..a143ab627 100644 --- a/src/panels/ConfigPanel/ConfigPanel.test.tsx +++ b/src/panels/ConfigPanel/ConfigPanel.test.tsx @@ -1,5 +1,6 @@ import { screen, waitFor, within } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import type { Mock } from "vitest"; import { vi } from "vitest"; @@ -50,6 +51,14 @@ vi.mock("juju/api-hooks/secrets", () => { }; }); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("ConfigPanel", () => { let state: RootState; const params = new URLSearchParams({ @@ -61,10 +70,9 @@ describe("ConfigPanel", () => { const url = `/models/eggman@external/hadoopspark?${params.toString()}`; const path = "/models/:userName/:modelName"; let getApplicationConfig: Mock; - const consoleError = console.error; beforeEach(() => { - console.error = vi.fn(); + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); vi.resetModules(); vi.spyOn(secretHooks, "useListSecrets").mockImplementation(() => vi.fn()); state = rootStateFactory.build({ @@ -140,7 +148,6 @@ describe("ConfigPanel", () => { }); afterEach(() => { - console.error = consoleError; vi.restoreAllMocks(); }); @@ -395,7 +402,7 @@ describe("ConfigPanel", () => { renderComponent(, { state, path, url }); expect(getApplicationConfig).toHaveBeenCalledTimes(1); await waitFor(() => { - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ConfigPanelLabel.GET_CONFIG_ERROR, new Error("Error while calling getApplicationConfig"), ); @@ -451,7 +458,7 @@ describe("ConfigPanel", () => { }), ); await waitFor(() => - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ConfirmationDialogLabel.SUBMIT_TO_JUJU_ERROR, new Error("Error while trying to save"), ), diff --git a/src/panels/ConfigPanel/ConfigPanel.tsx b/src/panels/ConfigPanel/ConfigPanel.tsx index c1d54f959..4641c947b 100644 --- a/src/panels/ConfigPanel/ConfigPanel.tsx +++ b/src/panels/ConfigPanel/ConfigPanel.tsx @@ -2,6 +2,7 @@ import type { ListSecretResult } from "@canonical/jujulib/dist/api/facades/secre import { ActionButton, Button } from "@canonical/react-components"; import classnames from "classnames"; import cloneDeep from "clone-deep"; +import log from "loglevel"; import type { MouseEvent } from "react"; import { useCallback, useEffect, useRef, useState } from "react"; import { useParams } from "react-router"; @@ -361,7 +362,7 @@ function getConfig( }) .catch((error) => { setInlineError(InlineErrors.GET_CONFIG, Label.GET_CONFIG_ERROR); - console.error(Label.GET_CONFIG_ERROR, error); + log.error(Label.GET_CONFIG_ERROR, error); }) .finally(() => setIsLoading(false)); } diff --git a/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.test.tsx b/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.test.tsx index 082914a24..81c3292a9 100644 --- a/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.test.tsx +++ b/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.test.tsx @@ -1,5 +1,6 @@ import { screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; +import log from "loglevel"; import type { Mock } from "vitest"; import { vi } from "vitest"; @@ -23,6 +24,14 @@ import { ConfigConfirmType } from "../types"; import ConfirmationDialog from "./ConfirmationDialog"; import { InlineErrors, Label, Label as ConfirmationDialogLabel } from "./types"; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("ConfirmationDialog", () => { let state: RootState; const params = new URLSearchParams({ @@ -32,7 +41,6 @@ describe("ConfirmationDialog", () => { panel: "config", }); const url = `/models/eggman@external/hadoopspark?${params.toString()}`; - const consoleError = console.error; let mockSetConfirmType: Mock; let mockSetInlineError: Mock; let mockHandleRemovePanelQueryParams: Mock; @@ -71,7 +79,7 @@ describe("ConfirmationDialog", () => { }; beforeEach(() => { - console.error = vi.fn(); + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); mockSetConfirmType = vi.fn(); mockSetInlineError = vi.fn(); mockHandleRemovePanelQueryParams = vi.fn(); @@ -86,7 +94,6 @@ describe("ConfirmationDialog", () => { }); afterEach(() => { - console.error = consoleError; vi.restoreAllMocks(); }); @@ -170,7 +177,7 @@ describe("ConfirmationDialog", () => { default: "eggman", }), }); - expect(console.error).not.toHaveBeenCalled(); + expect(log.error).not.toHaveBeenCalled(); expect(mockHandleRemovePanelQueryParams).toHaveBeenCalledOnce(); }); @@ -222,7 +229,7 @@ describe("ConfirmationDialog", () => { default: "eggman", }), }); - expect(console.error).not.toHaveBeenCalled(); + expect(log.error).not.toHaveBeenCalled(); expect(mockSetConfirmType).toHaveBeenCalledWith(ConfigConfirmType.GRANT); }); @@ -269,7 +276,7 @@ describe("ConfirmationDialog", () => { }), }); await waitFor(() => - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( Label.SUBMIT_TO_JUJU_ERROR, new Error("Error while trying to save"), ), @@ -368,7 +375,7 @@ describe("ConfirmationDialog", () => { expect(mockSetConfirmType).toHaveBeenCalledWith(null); expect(mockSetInlineError).toHaveBeenCalledWith(InlineErrors.FORM, null); expect(grantSecret).toHaveBeenCalledWith("secret:aabbccdd", ["easyrsa"]); - expect(console.error).not.toHaveBeenCalled(); + expect(log.error).not.toHaveBeenCalled(); expect(mockHandleRemovePanelQueryParams).toHaveBeenCalledOnce(); }); @@ -412,7 +419,7 @@ describe("ConfirmationDialog", () => { expect(mockSetInlineError).toHaveBeenCalledWith(InlineErrors.FORM, null); expect(grantSecret).toHaveBeenCalledWith("secret:aabbccdd", ["easyrsa"]); await waitFor(() => - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( Label.GRANT_ERROR, new Error("Caught error"), ), diff --git a/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.tsx b/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.tsx index b26516ee1..069504e8e 100644 --- a/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.tsx +++ b/src/panels/ConfigPanel/ConfirmationDialog/ConfirmationDialog.tsx @@ -1,4 +1,5 @@ import { ConfirmationModal } from "@canonical/react-components"; +import log from "loglevel"; import { useParams } from "react-router"; import usePortal from "react-useportal"; @@ -109,7 +110,7 @@ const ConfirmationDialog = ({ InlineErrors.SUBMIT_TO_JUJU, Label.SUBMIT_TO_JUJU_ERROR, ); - console.error(Label.SUBMIT_TO_JUJU_ERROR, error); + log.error(Label.SUBMIT_TO_JUJU_ERROR, error); }); }} close={() => setConfirmType(null)} @@ -149,7 +150,7 @@ const ConfirmationDialog = ({ handleRemovePanelQueryParams(); } catch (error) { setInlineError(InlineErrors.SUBMIT_TO_JUJU, Label.GRANT_ERROR); - console.error(Label.GRANT_ERROR, error); + log.error(Label.GRANT_ERROR, error); } })(); }} diff --git a/src/store/app/thunks.test.ts b/src/store/app/thunks.test.ts index 323031c88..876feeb8a 100644 --- a/src/store/app/thunks.test.ts +++ b/src/store/app/thunks.test.ts @@ -1,3 +1,4 @@ +import log from "loglevel"; import { vi } from "vitest"; import { pollWhoamiStop } from "juju/jimm/listeners"; @@ -20,15 +21,22 @@ import type { WindowConfig } from "types"; import { logOut, connectAndStartPolling } from "./thunks"; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("thunks", () => { - const consoleError = console.error; let state: RootState; beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); window.jujuDashboardConfig = { controllerAPIEndpoint: "wss://example.com/api", } as WindowConfig; - console.error = vi.fn(); fetchMock.resetMocks(); state = rootStateFactory.build({ general: generalStateFactory.build({ @@ -64,7 +72,6 @@ describe("thunks", () => { }); afterEach(() => { - console.error = consoleError; delete window.jujuDashboardConfig; }); @@ -154,7 +161,7 @@ describe("thunks", () => { isJuju: true, }), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Error while triggering the connection and polling of models.", "Error while dispatching connectAndPollControllers!", ); @@ -185,7 +192,7 @@ describe("thunks", () => { isJuju: true, }), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Error while triggering the connection and polling of models.", new Error("Error while dispatching connectAndPollControllers!"), ); @@ -217,7 +224,7 @@ describe("thunks", () => { isJuju: true, }), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Error while triggering the connection and polling of models.", ["Unknown error."], ); diff --git a/src/store/app/thunks.ts b/src/store/app/thunks.ts index 562a3ca79..cae2977f6 100644 --- a/src/store/app/thunks.ts +++ b/src/store/app/thunks.ts @@ -1,4 +1,5 @@ import { createAsyncThunk } from "@reduxjs/toolkit"; +import log from "loglevel"; import bakery from "juju/bakery"; import { pollWhoamiStop } from "juju/jimm/listeners"; @@ -84,7 +85,7 @@ export const connectAndStartPolling = createAsyncThunk< } catch (error) { // a common error logged to the console by this is: // Error while triggering the connection and polling of models. cannot send request {"type":"ModelManager","request":"ListModels","version":5,"params":...}: connection state 3 is not open - console.error( + log.error( "Error while triggering the connection and polling of models.", error, ); diff --git a/src/store/middleware/check-auth.test.ts b/src/store/middleware/check-auth.test.ts index 6b3ebdbb9..398e1a265 100644 --- a/src/store/middleware/check-auth.test.ts +++ b/src/store/middleware/check-auth.test.ts @@ -1,3 +1,4 @@ +import log from "loglevel"; import type { UnknownAction, MiddlewareAPI } from "redux"; import type { Mock } from "vitest"; import { vi } from "vitest"; @@ -14,17 +15,24 @@ import { import { checkAuthMiddleware } from "./check-auth"; +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("model poller", () => { let fakeStore: MiddlewareAPI; let next: Mock; const originalLog = console.log; - const originalError = console.error; const wsControllerURL = "wss://example.com"; let state: RootState; beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); console.log = vi.fn(); - console.error = vi.fn(); vi.useFakeTimers(); next = vi.fn(); state = rootStateFactory.build({ @@ -61,7 +69,6 @@ describe("model poller", () => { afterEach(() => { console.log = originalLog; - console.error = originalError; vi.restoreAllMocks(); vi.useRealTimers(); }); diff --git a/src/store/middleware/check-auth.ts b/src/store/middleware/check-auth.ts index c6cf3151f..ae9bb9a38 100644 --- a/src/store/middleware/check-auth.ts +++ b/src/store/middleware/check-auth.ts @@ -3,6 +3,7 @@ has been allowed. */ +import log from "loglevel"; import { isAction, type Middleware } from "redux"; import * as jimmListeners from "juju/jimm/listeners"; @@ -17,7 +18,7 @@ import { isPayloadAction } from "types"; function error(name: string, wsControllerURL?: string | null) { // Not shown in UI. Logged for debugging purposes. - console.error( + log.error( "Unable to perform action: ", name, wsControllerURL @@ -32,7 +33,7 @@ function error(name: string, wsControllerURL?: string | null) { export const checkLoggedIn = (state: RootState, wsControllerURL: string) => { if (!wsControllerURL) { // Not shown in UI. Logged for debugging purposes. - console.error( + log.error( "Unable to determine logged in status. " + "'wsControllerURL' was not provided in the action that was dispatched.", ); diff --git a/src/store/middleware/model-poller.test.ts b/src/store/middleware/model-poller.test.ts index 553b0f044..3bb3d25fb 100644 --- a/src/store/middleware/model-poller.test.ts +++ b/src/store/middleware/model-poller.test.ts @@ -1,4 +1,5 @@ import type { Client, Connection, Transport } from "@canonical/jujulib"; +import log from "loglevel"; import type { UnknownAction, MiddlewareAPI } from "redux"; import type { Mock } from "vitest"; import { vi } from "vitest"; @@ -40,6 +41,14 @@ vi.mock("juju/jimm/api", () => ({ findAuditEvents: vi.fn(), })); +vi.mock("loglevel", async () => { + const actual = await vi.importActual("loglevel"); + return { + ...actual, + error: vi.fn(), + }; +}); + describe("model poller", () => { let fakeStore: MiddlewareAPI; let next: Mock; @@ -82,9 +91,9 @@ describe("model poller", () => { }, }, }); - const consoleError = console.error; beforeEach(() => { + vi.spyOn(log, "error").mockImplementation(() => vi.fn()); vi.useFakeTimers(); next = vi.fn(); fakeStore = { @@ -112,11 +121,6 @@ describe("model poller", () => { juju = { logout: vi.fn(), } as unknown as Client; - console.error = vi.fn(); - }); - - afterEach(() => { - console.error = consoleError; }); const runMiddleware = async (actionOverrides?: Partial) => { @@ -491,7 +495,7 @@ describe("model poller", () => { new Error(ModelsError.LOAD_ALL_MODELS), ); await runMiddleware(); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ModelsError.LOAD_ALL_MODELS, new Error(ModelsError.LOAD_ALL_MODELS), ); @@ -514,7 +518,7 @@ describe("model poller", () => { new Error(ModelsError.LOAD_SOME_MODELS), ); await runMiddleware(); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ModelsError.LOAD_SOME_MODELS, new Error(ModelsError.LOAD_SOME_MODELS), ); @@ -545,11 +549,11 @@ describe("model poller", () => { ); runMiddleware({ payload: { controllers, isJuju: true, poll: 1 }, - }).catch(console.error); + }).catch(log.error); vi.advanceTimersByTime(30000); // Resolve the async calls again. await vi.runAllTimersAsync(); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ModelsError.LOAD_LATEST_MODELS, new Error(ModelsError.LOAD_SOME_MODELS), ); @@ -574,7 +578,7 @@ describe("model poller", () => { }), ); await runMiddleware(); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( ModelsError.LIST_OR_UPDATE_MODELS, new Error(ModelsError.LIST_OR_UPDATE_MODELS), ); @@ -595,7 +599,7 @@ describe("model poller", () => { juju, })); runMiddleware({ payload: { controllers, isJuju: true, poll: 1 } }).catch( - console.error, + log.error, ); vi.advanceTimersByTime(30000); // Resolve the async calls again. @@ -732,7 +736,7 @@ describe("model poller", () => { expect(fakeStore.dispatch).toHaveBeenCalledWith( jujuActions.updateAuditEventsErrors("Uh oh!"), ); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Could not fetch audit events.", new Error("Uh oh!"), ); @@ -822,7 +826,7 @@ describe("model poller", () => { query: ".", }); await middleware(next)(action); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Could not perform cross model query.", new Error("Uh oh!"), ); @@ -846,7 +850,7 @@ describe("model poller", () => { query: ".", }); await middleware(next)(action); - expect(console.error).toHaveBeenCalledWith( + expect(log.error).toHaveBeenCalledWith( "Could not perform cross model query.", "Uh oh!", ); diff --git a/src/store/middleware/model-poller.ts b/src/store/middleware/model-poller.ts index 9df946b8a..d3c755a95 100644 --- a/src/store/middleware/model-poller.ts +++ b/src/store/middleware/model-poller.ts @@ -1,6 +1,7 @@ import type { Client } from "@canonical/jujulib"; import { unwrapResult } from "@reduxjs/toolkit"; import * as Sentry from "@sentry/browser"; +import log from "loglevel"; import { isAction, type Middleware } from "redux"; import { @@ -235,7 +236,7 @@ export const modelPollerMiddleware: Middleware< } else { errorMessage = ModelsError.LIST_OR_UPDATE_MODELS; } - console.error(errorMessage, error); + log.error(errorMessage, error); reduxStore.dispatch( jujuActions.updateModelsError({ modelsError: errorMessage, @@ -305,7 +306,7 @@ export const modelPollerMiddleware: Middleware< const auditEvents = await findAuditEvents(conn, params); reduxStore.dispatch(jujuActions.updateAuditEvents(auditEvents.events)); } catch (error) { - console.error("Could not fetch audit events.", error); + log.error("Could not fetch audit events.", error); reduxStore.dispatch( jujuActions.updateAuditEventsErrors(toErrorString(error)), ); @@ -341,7 +342,7 @@ export const modelPollerMiddleware: Middleware< ), ); } catch (error) { - console.error("Could not perform cross model query.", error); + log.error("Could not perform cross model query.", error); const errorMessage = error instanceof Error ? error.message diff --git a/src/store/store.ts b/src/store/store.ts index b3e38f70e..b51377709 100644 --- a/src/store/store.ts +++ b/src/store/store.ts @@ -1,5 +1,6 @@ import type { UnknownAction } from "@reduxjs/toolkit"; import { combineReducers, configureStore } from "@reduxjs/toolkit"; +import log from "loglevel"; import { useCallback } from "react"; import type { TypedUseSelectorHook } from "react-redux"; import { useDispatch, useSelector, useStore } from "react-redux"; @@ -16,7 +17,7 @@ if (!import.meta.env.PROD && process.env.VITE_APP_MOCK_STORE) { try { preloadedState = JSON.parse(process.env.VITE_APP_MOCK_STORE); } catch (error) { - console.error("VITE_APP_MOCK_STORE could not be parsed"); + log.error("VITE_APP_MOCK_STORE could not be parsed"); } } diff --git a/yarn.lock b/yarn.lock index 7dab55d8a..7d799b909 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8357,6 +8357,7 @@ __metadata: jest-websocket-mock: "npm:2.5.0" lodash.isequal: "npm:4.5.0" lodash.mergewith: "npm:4.6.2" + loglevel: "npm:^1.9.2" npm-package-json-lint: "npm:8.0.0" postcss: "npm:8.4.49" prettier: "npm:3.4.2" @@ -8534,7 +8535,7 @@ __metadata: languageName: node linkType: hard -"loglevel@npm:1.9.2": +"loglevel@npm:1.9.2, loglevel@npm:^1.9.2": version: 1.9.2 resolution: "loglevel@npm:1.9.2" checksum: 10c0/1e317fa4648fe0b4a4cffef6de037340592cee8547b07d4ce97a487abe9153e704b98451100c799b032c72bb89c9366d71c9fb8192ada8703269263ae77acdc7