Skip to content

Commit

Permalink
feat(logging): Use loglevel library for error handling
Browse files Browse the repository at this point in the history
  • Loading branch information
Ninfa-Jeon committed Feb 4, 2025
1 parent 3b25638 commit aca9cda
Show file tree
Hide file tree
Showing 39 changed files with 260 additions and 180 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
17 changes: 10 additions & 7 deletions src/components/App/App.test.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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();
});
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;
});
});
20 changes: 14 additions & 6 deletions src/components/LogIn/UserPassForm/UserPassForm.test.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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();
});
Expand Down Expand Up @@ -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" }),
);
Expand All @@ -82,11 +92,9 @@ describe("UserPassForm", () => {
renderComponent(<UserPassForm />);
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;
});
});
3 changes: 2 additions & 1 deletion src/components/LogIn/UserPassForm/UserPassForm.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { unwrapResult } from "@reduxjs/toolkit";
import log from "loglevel";
import type { FormEvent } from "react";

import bakery from "juju/bakery";
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) => log.error(Label.POLLING_ERROR, error));
}
}

Expand Down
22 changes: 15 additions & 7 deletions src/components/ShareCard/ShareCard.test.tsx
Original file line number Diff line number Diff line change
@@ -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(
<ShareCard
Expand Down Expand Up @@ -74,10 +87,7 @@ describe("Share Card", () => {
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(
Expand All @@ -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;
});
});
3 changes: 2 additions & 1 deletion src/components/ShareCard/ShareCard.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -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}
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
19 changes: 12 additions & 7 deletions src/hooks/useLocalStorage.test.ts
Original file line number Diff line number Diff line change
@@ -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();
});

Expand All @@ -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),
);
Expand Down Expand Up @@ -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");
Expand Down
5 changes: 3 additions & 2 deletions src/hooks/useLocalStorage.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import log from "loglevel";
import { useState } from "react";

function useLocalStorage<V>(
Expand All @@ -12,7 +13,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);
log.error("Unable to parse local storage:", error);
return initialValue;
}
});
Expand All @@ -26,7 +27,7 @@ function useLocalStorage<V>(
window.localStorage.setItem(key, stringified);
} catch (error) {
// Not shown in UI. Logged for debugging purposes.
console.error(error);
log.error(error);
}
};

Expand Down
17 changes: 10 additions & 7 deletions src/hooks/useLogout.test.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -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 () => {
Expand Down
3 changes: 2 additions & 1 deletion src/hooks/useLogout.tsx
Original file line number Diff line number Diff line change
@@ -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";
Expand Down Expand Up @@ -30,7 +31,7 @@ const useLogout = () => {
</>
</ToastCard>
));
console.error(Label.LOGOUT_ERROR, error);
log.error(Label.LOGOUT_ERROR, error);
});
};
};
Expand Down
23 changes: 12 additions & 11 deletions src/index.test.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { screen, waitFor } from "@testing-library/dom";
import log from "loglevel";
import type { UnknownAction } from "redux";
import { vi } from "vitest";

Expand Down Expand Up @@ -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(() => {
Expand All @@ -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);
Expand All @@ -59,7 +67,6 @@ describe("renderApp", () => {
enumerable: true,
value: windowLocation,
});
console.error = consoleError;
if (rootNode) {
document.body.removeChild(rootNode);
}
Expand Down Expand Up @@ -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", () => {
Expand Down Expand Up @@ -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!"),
),
Expand Down
Loading

0 comments on commit aca9cda

Please sign in to comment.