Skip to content

Commit

Permalink
feat(logging): Use loglevel library for error handling (#1845)
Browse files Browse the repository at this point in the history
  • Loading branch information
Ninfa-Jeon authored Feb 10, 2025
1 parent 44d98b1 commit 5401e10
Show file tree
Hide file tree
Showing 43 changed files with 86 additions and 388 deletions.
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
10 changes: 0 additions & 10 deletions src/components/App/App.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,17 +29,7 @@ vi.mock("react-router", async () => {
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();
});

afterEach(() => {
console.error = consoleError;
vi.resetAllMocks();
vi.unstubAllEnvs();
});
Expand Down
5 changes: 0 additions & 5 deletions src/components/LogIn/LogIn.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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({
Expand All @@ -186,7 +183,5 @@ describe("LogIn", () => {
{ pointerEventsCheck: 0 },
);
expect(screen.queryByTestId("toast-card")).not.toBeInTheDocument();

console.error = consoleError;
});
});
38 changes: 0 additions & 38 deletions src/components/LogIn/UserPassForm/UserPassForm.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,9 @@ import { generalStateFactory } from "testing/factories/general";
import { rootStateFactory } from "testing/factories/root";
import { renderComponent } from "testing/utils";

import { Label } from "../types";

import UserPassForm from "./UserPassForm";

describe("UserPassForm", () => {
afterEach(() => {
vi.restoreAllMocks();
});

it("should log in", async () => {
// Mock the result of the thunk to be a normal action so that it can be tested
// for. This is necessary because we don't have a full store set up which
Expand Down Expand Up @@ -57,36 +51,4 @@ describe("UserPassForm", () => {
type: "connectAndStartPolling",
});
});

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" }),
);
vi.spyOn(dashboardStore, "useAppDispatch").mockImplementation(
vi
.fn()
.mockReturnValue((action: unknown) =>
action instanceof Object &&
"type" in action &&
action.type === "connectAndStartPolling"
? Promise.reject(
new Error("Error while dispatching connectAndStartPolling!"),
)
: null,
),
);

renderComponent(<UserPassForm />);
await userEvent.click(screen.getByRole("button"));
expect(appThunks.connectAndStartPolling).toHaveBeenCalledTimes(1);
expect(console.error).toHaveBeenCalledWith(
Label.POLLING_ERROR,
new Error("Error while dispatching connectAndStartPolling!"),
);

console.error = consoleError;
});
});
3 changes: 2 additions & 1 deletion src/components/LogIn/UserPassForm/UserPassForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { thunks as appThunks } from "store/app";
import { actions as generalActions } from "store/general";
import { getWSControllerURL } from "store/general/selectors";
import { useAppDispatch, useAppSelector } from "store/store";
import { logger } from "utils/logger";

import { Label } from "../types";

Expand Down Expand Up @@ -35,7 +36,7 @@ const UserPassForm = () => {
if (bakery) {
dispatch(appThunks.connectAndStartPolling())
.then(unwrapResult)
.catch((error) => console.error(Label.POLLING_ERROR, error));
.catch((error) => logger.error(Label.POLLING_ERROR, error));
}
}

Expand Down
25 changes: 0 additions & 25 deletions src/components/ShareCard/ShareCard.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -73,29 +73,4 @@ describe("Share Card", () => {
await userEvent.selectOptions(screen.getByRole("combobox"), "write");
expect(accessSelectChangeFn).toHaveBeenCalled();
});

it("should display console error when trying to change access", async () => {
const consoleError = console.error;
console.error = vi.fn();

const removeUserFn = vi.fn();
const accessSelectChangeFn = vi.fn(() => Promise.reject(new Error()));
render(
<ShareCard
userName="janedoe"
lastConnected="2021-06-03T16:03:15Z"
access="read"
isOwner={false}
removeUser={removeUserFn}
accessSelectChange={accessSelectChangeFn}
/>,
);
await userEvent.selectOptions(screen.getByRole("combobox"), "write");
expect(console.error).toHaveBeenCalledWith(
Label.ACCESS_CHANGE_ERROR,
new Error(),
);

console.error = consoleError;
});
});
3 changes: 2 additions & 1 deletion src/components/ShareCard/ShareCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { useEffect, useState } from "react";
import SlideDownFadeOut from "animations/SlideDownFadeOut";
import TruncatedTooltip from "components/TruncatedTooltip";
import { formatFriendlyDateToNow } from "components/utils";
import { logger } from "utils/logger";

import "./_share-card.scss";
import { Label } from "./types";
Expand Down Expand Up @@ -117,7 +118,7 @@ export default function ShareCard({
return;
})
.catch((error) =>
console.error(Label.ACCESS_CHANGE_ERROR, error),
logger.error(Label.ACCESS_CHANGE_ERROR, error),
);
}}
value={access}
Expand Down
3 changes: 0 additions & 3 deletions src/components/UserMenu/UserMenu.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import UserMenu from "./UserMenu";

describe("User Menu", () => {
let state: RootState;
const consoleError = console.error;

beforeEach(() => {
state = rootStateFactory.build({
Expand All @@ -33,12 +32,10 @@ describe("User Menu", () => {
},
}),
});
console.error = vi.fn();
});

afterEach(() => {
vi.restoreAllMocks();
console.error = consoleError;
});

it("is inactive by default", () => {
Expand Down
14 changes: 0 additions & 14 deletions src/hooks/useLocalStorage.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,9 @@
import { act, renderHook } from "@testing-library/react";
import { vi } from "vitest";

import useLocalStorage from "./useLocalStorage";

describe("useLocalStorage", () => {
let consoleError: Console["error"];

beforeEach(() => {
consoleError = console.error;
console.error = vi.fn();
});

afterEach(() => {
console.error = consoleError;
localStorage.clear();
});

Expand All @@ -38,10 +29,6 @@ describe("useLocalStorage", () => {
const { result } = renderHook(() =>
useLocalStorage("test-key", "init-val"),
);
expect(console.error).toHaveBeenCalledWith(
"Unable to parse local storage:",
expect.any(Error),
);
const [value] = result.current;
expect(value).toBe("init-val");
});
Expand Down Expand Up @@ -71,7 +58,6 @@ describe("useLocalStorage", () => {
act(() => {
setValue(circular as unknown as string);
});
expect(console.error).toHaveBeenCalledWith(expect.any(Error));
const [value] = result.current;
expect(value).toBe("init-val");
expect(JSON.parse(localStorage.getItem("test-key") ?? "")).toBe("init-val");
Expand Down
6 changes: 4 additions & 2 deletions src/hooks/useLocalStorage.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import { useState } from "react";

import { logger } from "utils/logger";

function useLocalStorage<V>(
key: string,
initialValue: V,
Expand All @@ -12,7 +14,7 @@ function useLocalStorage<V>(
return item ? JSON.parse(item) : initialValue;
} catch (error) {
// Not shown in UI. Logged for debugging purposes.
console.error("Unable to parse local storage:", error);
logger.error("Unable to parse local storage:", error);
return initialValue;
}
});
Expand All @@ -26,7 +28,7 @@ function useLocalStorage<V>(
window.localStorage.setItem(key, stringified);
} catch (error) {
// Not shown in UI. Logged for debugging purposes.
console.error(error);
logger.error(error);
}
};

Expand Down
10 changes: 0 additions & 10 deletions src/hooks/useLogout.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,6 @@ import { renderComponent, renderWrappedHook } from "testing/utils";
import useLogout from "./useLogout";

describe("useLogout", () => {
const consoleError = console.error;

beforeEach(() => {
console.error = vi.fn();
});

afterEach(() => {
console.error = consoleError;
});

it("should logout", async () => {
vi.spyOn(appThunks, "logOut").mockImplementation(
vi.fn().mockReturnValue({ type: "logOut", catch: vi.fn() }),
Expand Down
3 changes: 2 additions & 1 deletion src/hooks/useLogout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import reactHotToast from "react-hot-toast";
import ToastCard from "components/ToastCard";
import { thunks as appThunks } from "store/app";
import { useAppDispatch } from "store/store";
import { logger } from "utils/logger";

export enum Label {
LOGOUT_ERROR = "Error when trying to logout.",
Expand All @@ -30,7 +31,7 @@ const useLogout = () => {
</>
</ToastCard>
));
console.error(Label.LOGOUT_ERROR, error);
logger.error(Label.LOGOUT_ERROR, error);
});
};
};
Expand Down
4 changes: 3 additions & 1 deletion src/hooks/useQueryParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { useCallback } from "react";
import type { NavigateOptions } from "react-router";
import { useSearchParams } from "react-router";

import { logger } from "utils/logger";

export type SetParams<P> = (
params?: Partial<P> | null,
options?: NavigateOptions,
Expand Down Expand Up @@ -42,7 +44,7 @@ export const useQueryParams = <P extends QueryParams>(
const sanitizedKey = DOMPurify.sanitize(key);
if (key !== sanitizedKey) {
searchParams.delete(key);
console.log(
logger.log(
`Query param key "${key}" has been changed to "${sanitizedKey}"` +
" in order to prevent potential XSS Attacks.",
);
Expand Down
45 changes: 0 additions & 45 deletions src/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { screen, waitFor } from "@testing-library/dom";
import type { UnknownAction } from "redux";
import { vi } from "vitest";

import * as storeModule from "store";
Expand Down Expand Up @@ -37,7 +36,6 @@ const appVersion = packageJSON.version;

describe("renderApp", () => {
let rootNode: HTMLElement;
const consoleError = console.error;
const windowLocation = window.location;

beforeEach(() => {
Expand All @@ -46,8 +44,6 @@ describe("renderApp", () => {
enumerable: true,
value: new URL(window.location.href),
});
// Hide the config errors from the test output.
console.error = vi.fn();
rootNode = document.createElement("div");
rootNode.setAttribute("id", ROOT_ID);
document.body.appendChild(rootNode);
Expand All @@ -59,7 +55,6 @@ describe("renderApp", () => {
enumerable: true,
value: windowLocation,
});
console.error = consoleError;
if (rootNode) {
document.body.removeChild(rootNode);
}
Expand Down Expand Up @@ -250,16 +245,6 @@ describe("renderApp", () => {
});

describe("getControllerAPIEndpointErrors", () => {
const consoleError = console.error;

beforeEach(() => {
console.error = vi.fn();
});

afterEach(() => {
console.error = consoleError;
});

it("should handle secure protocol", () => {
expect(
getControllerAPIEndpointErrors("wss://example.com:80/api"),
Expand Down Expand Up @@ -305,34 +290,4 @@ describe("getControllerAPIEndpointErrors", () => {
"controllerAPIEndpoint (example.com:80/api) must be an absolute path or begin with ws:// or wss://.",
);
});

it("should show console error when dispatching connectAndStartPolling", async () => {
vi.spyOn(appThunks, "connectAndStartPolling").mockImplementation(
vi.fn().mockReturnValue({ type: "connectAndStartPolling" }),
);
vi.spyOn(storeModule.default, "dispatch").mockImplementation(
(action) =>
(action instanceof Object &&
"type" in action &&
action.type === "connectAndStartPolling"
? Promise.reject(
new Error("Error while dispatching connectAndStartPolling!"),
)
: null) as unknown as UnknownAction,
);
const config = configFactory.build({
baseControllerURL: null,
identityProviderURL: "/candid",
isJuju: true,
});
window.jujuDashboardConfig = config;
renderApp();
expect(appThunks.connectAndStartPolling).toHaveBeenCalledTimes(1);
await waitFor(() =>
expect(console.error).toHaveBeenCalledWith(
Label.POLLING_ERROR,
new Error("Error while dispatching connectAndStartPolling!"),
),
);
});
});
Loading

0 comments on commit 5401e10

Please sign in to comment.