From c546042d18c6c248d24f53bc8efa3710286f4bbc Mon Sep 17 00:00:00 2001 From: David Baker Date: Mon, 25 Sep 2023 18:04:34 +0100 Subject: [PATCH] Use the loglevel library's extensions ...instead of monkey patching the console log objects. We use a logging framework everywhere now (this fixes the times when we didn't...) so there's not really a reason to do this the hacky way anymore. This means that log lines now appear to come from whatever else is intercepting the logger (eg. sentry) rather than rageshake.ts. Opinions on this welcome on whether it's better or not. --- .eslintrc.cjs | 2 + src/FullScreenView.tsx | 3 +- src/auth/RegisterPage.tsx | 3 +- src/auth/useRecaptcha.ts | 3 +- src/home/RegisteredView.tsx | 3 +- src/home/UnauthenticatedView.tsx | 3 +- src/main.tsx | 5 +- src/matrix-utils.ts | 2 +- src/profile/useProfile.ts | 3 +- src/room/RoomAuthView.tsx | 3 +- src/room/RoomPage.tsx | 3 +- src/room/VideoPreview.tsx | 3 +- src/settings/rageshake.ts | 80 ++++++++++++++++++-------------- src/settings/submit-rageshake.ts | 3 +- src/video-grid/VideoGrid.tsx | 3 +- 15 files changed, 72 insertions(+), 50 deletions(-) diff --git a/.eslintrc.cjs b/.eslintrc.cjs index 173d8dca..ac9f913f 100644 --- a/.eslintrc.cjs +++ b/.eslintrc.cjs @@ -33,6 +33,8 @@ module.exports = { rules: { // We're aiming to convert this code to strict mode "@typescript-eslint/no-non-null-assertion": "off", + // We should use the js-sdk logger, never console directly. + "no-console": ["error"], }, }, ], diff --git a/src/FullScreenView.tsx b/src/FullScreenView.tsx index 6ac134b8..5ce2694c 100644 --- a/src/FullScreenView.tsx +++ b/src/FullScreenView.tsx @@ -19,6 +19,7 @@ import { useLocation } from "react-router-dom"; import classNames from "classnames"; import { Trans, useTranslation } from "react-i18next"; import * as Sentry from "@sentry/react"; +import { logger } from "matrix-js-sdk/src/logger"; import { Header, HeaderLogo, LeftNav, RightNav } from "./Header"; import { LinkButton, Button } from "./button"; @@ -57,7 +58,7 @@ export function ErrorView({ error }: ErrorViewProps) { const { t } = useTranslation(); useEffect(() => { - console.error(error); + logger.error(error); Sentry.captureException(error); }, [error]); diff --git a/src/auth/RegisterPage.tsx b/src/auth/RegisterPage.tsx index c49d9b43..d0c22fc8 100644 --- a/src/auth/RegisterPage.tsx +++ b/src/auth/RegisterPage.tsx @@ -27,6 +27,7 @@ import { useHistory, useLocation } from "react-router-dom"; import { captureException } from "@sentry/react"; import { sleep } from "matrix-js-sdk/src/utils"; import { Trans, useTranslation } from "react-i18next"; +import { logger } from "matrix-js-sdk/src/logger"; import { FieldRow, InputField, ErrorMessage } from "../input/Input"; import { Button } from "../button"; @@ -97,7 +98,7 @@ export const RegisterPage: FC = () => { await newClient.joinRoom(roomId); } else { captureException(error); - console.error(`Couldn't join room ${roomId}`, error); + logger.error(`Couldn't join room ${roomId}`, error); } } } diff --git a/src/auth/useRecaptcha.ts b/src/auth/useRecaptcha.ts index 647370da..0ee9f33b 100644 --- a/src/auth/useRecaptcha.ts +++ b/src/auth/useRecaptcha.ts @@ -17,6 +17,7 @@ limitations under the License. import { useEffect, useCallback, useRef, useState } from "react"; import { randomString } from "matrix-js-sdk/src/randomstring"; import { useTranslation } from "react-i18next"; +import { logger } from "matrix-js-sdk/src/logger"; import { translatedError } from "../TranslatedError"; @@ -74,7 +75,7 @@ export const useRecaptcha = (sitekey?: string) => { } if (!window.grecaptcha) { - console.log("Recaptcha not loaded"); + logger.log("Recaptcha not loaded"); return Promise.reject(translatedError("Recaptcha not loaded", t)); } diff --git a/src/home/RegisteredView.tsx b/src/home/RegisteredView.tsx index 5552eda3..4bba6e43 100644 --- a/src/home/RegisteredView.tsx +++ b/src/home/RegisteredView.tsx @@ -20,6 +20,7 @@ import { MatrixClient } from "matrix-js-sdk/src/client"; import { randomString } from "matrix-js-sdk/src/randomstring"; import { useTranslation } from "react-i18next"; import { Heading } from "@vector-im/compound-web"; +import { logger } from "matrix-js-sdk/src/logger"; import { createRoom, @@ -97,7 +98,7 @@ export function RegisteredView({ client }: Props) { setError(undefined); setJoinExistingCallModalOpen(true); } else { - console.error(error); + logger.error(error); setLoading(false); setError(error); } diff --git a/src/home/UnauthenticatedView.tsx b/src/home/UnauthenticatedView.tsx index 8cd64c14..ad44536b 100644 --- a/src/home/UnauthenticatedView.tsx +++ b/src/home/UnauthenticatedView.tsx @@ -19,6 +19,7 @@ import { useHistory } from "react-router-dom"; import { randomString } from "matrix-js-sdk/src/randomstring"; import { Trans, useTranslation } from "react-i18next"; import { Heading } from "@vector-im/compound-web"; +import { logger } from "matrix-js-sdk/src/logger"; import { useClient } from "../ClientContext"; import { Header, HeaderLogo, LeftNav, RightNav } from "../Header"; @@ -130,7 +131,7 @@ export const UnauthenticatedView: FC = () => { } submit().catch((error) => { - console.error(error); + logger.error(error); setLoading(false); setError(error); reset(); diff --git a/src/main.tsx b/src/main.tsx index d04a3a94..58b653c8 100644 --- a/src/main.tsx +++ b/src/main.tsx @@ -23,15 +23,16 @@ import "matrix-js-sdk/src/browser-index"; import { StrictMode } from "react"; import { createRoot } from "react-dom/client"; import { createBrowserHistory } from "history"; - import "./index.css"; +import { logger } from "matrix-js-sdk/src/logger"; + import App from "./App"; import { init as initRageshake } from "./settings/rageshake"; import { Initializer } from "./initializer"; initRageshake(); -console.info(`Element Call ${import.meta.env.VITE_APP_VERSION || "dev"}`); +logger.info(`Element Call ${import.meta.env.VITE_APP_VERSION || "dev"}`); const root = createRoot(document.getElementById("root")!); diff --git a/src/matrix-utils.ts b/src/matrix-utils.ts index a03b4bc4..e64d3d8d 100644 --- a/src/matrix-utils.ts +++ b/src/matrix-utils.ts @@ -177,7 +177,7 @@ export async function initClient( try { await client.store.startup(); } catch (error) { - console.error( + logger.error( "Error starting matrix client store. Falling back to memory store.", error ); diff --git a/src/profile/useProfile.ts b/src/profile/useProfile.ts index 2281bb1e..11c1ec74 100644 --- a/src/profile/useProfile.ts +++ b/src/profile/useProfile.ts @@ -19,6 +19,7 @@ import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { User, UserEvent } from "matrix-js-sdk/src/models/user"; import { FileType } from "matrix-js-sdk/src/http-api"; import { useState, useCallback, useEffect } from "react"; +import { logger } from "matrix-js-sdk/src/logger"; interface ProfileLoadState { success: boolean; @@ -127,7 +128,7 @@ export function useProfile(client: MatrixClient | undefined) { })); } } else { - console.error("Client not initialized before calling saveProfile"); + logger.error("Client not initialized before calling saveProfile"); } }, [client] diff --git a/src/room/RoomAuthView.tsx b/src/room/RoomAuthView.tsx index 28c1ec6c..67365241 100644 --- a/src/room/RoomAuthView.tsx +++ b/src/room/RoomAuthView.tsx @@ -17,6 +17,7 @@ limitations under the License. import { useCallback, useState } from "react"; import { useLocation } from "react-router-dom"; import { Trans, useTranslation } from "react-i18next"; +import { logger } from "matrix-js-sdk/src/logger"; import styles from "./RoomAuthView.module.css"; import { Button } from "../button"; @@ -46,7 +47,7 @@ export function RoomAuthView() { typeof dataForDisplayName === "string" ? dataForDisplayName : ""; registerPasswordlessUser(displayName).catch((error) => { - console.error("Failed to register passwordless user", e); + logger.error("Failed to register passwordless user", e); setLoading(false); setError(error); }); diff --git a/src/room/RoomPage.tsx b/src/room/RoomPage.tsx index 1a53dc49..ffc9562a 100644 --- a/src/room/RoomPage.tsx +++ b/src/room/RoomPage.tsx @@ -16,6 +16,7 @@ limitations under the License. import { FC, useEffect, useState, useCallback, ReactNode } from "react"; import { MatrixRTCSession } from "matrix-js-sdk/src/matrixrtc/MatrixRTCSession"; +import { logger } from "matrix-js-sdk/src/logger"; import { useClientLegacy } from "../ClientContext"; import { ErrorView, LoadingView } from "../FullScreenView"; @@ -37,7 +38,7 @@ export const RoomPage: FC = () => { const roomIdOrAlias = roomId ?? roomAlias; if (!roomIdOrAlias) { - console.error("No room specified"); + logger.error("No room specified"); } const [optInAnalytics, setOptInAnalytics] = useOptInAnalytics(); diff --git a/src/room/VideoPreview.tsx b/src/room/VideoPreview.tsx index fda5c3a4..36323ba9 100644 --- a/src/room/VideoPreview.tsx +++ b/src/room/VideoPreview.tsx @@ -24,6 +24,7 @@ import { Track, } from "livekit-client"; import classNames from "classnames"; +import { logger } from "matrix-js-sdk/src/logger"; import { Avatar } from "../Avatar"; import styles from "./VideoPreview.module.css"; @@ -80,7 +81,7 @@ export const VideoPreview: FC = ({ }, }, (error) => { - console.error("Error while creating preview Tracks:", error); + logger.error("Error while creating preview Tracks:", error); } ); const videoTrack = useMemo( diff --git a/src/settings/rageshake.ts b/src/settings/rageshake.ts index 8fcc19da..2b34f7d8 100644 --- a/src/settings/rageshake.ts +++ b/src/settings/rageshake.ts @@ -54,11 +54,6 @@ enum ConsoleLoggerEvent { Log = "log", } -type LogFunction = ( - ...args: (Error | DOMException | object | string)[] -) => void; -type LogFunctionName = "log" | "info" | "warn" | "error"; - // A class which monkey-patches the global console and stores log lines. interface LogEntry { @@ -69,37 +64,11 @@ interface LogEntry { class ConsoleLogger extends EventEmitter { private logs = ""; - private originalFunctions: { [key in LogFunctionName]?: LogFunction } = {}; - public monkeyPatch(consoleObj: Console): void { - // Monkey-patch console logging - const consoleFunctionsToLevels = { - log: "I", - info: "I", - warn: "W", - error: "E", - }; - - Object.entries(consoleFunctionsToLevels).forEach(([name, level]) => { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - const originalFn = consoleObj[name].bind(consoleObj); - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - this.originalFunctions[name] = originalFn; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - consoleObj[name] = (...args) => { - this.log(level, ...args); - originalFn(...args); - }; - }); - } - - public log( - level: string, + public log = ( + level: LogLevel, ...args: (Error | DOMException | object | string)[] - ): void { + ): void => { // We don't know what locale the user may be running so use ISO strings const ts = new Date().toISOString(); @@ -129,7 +98,7 @@ class ConsoleLogger extends EventEmitter { this.logs += line; this.emit(ConsoleLoggerEvent.Log); - } + }; /** * Returns the log lines to flush to disk and empties the internal log buffer @@ -510,7 +479,7 @@ declare global { */ export function init(): Promise { global.mx_rage_logger = new ConsoleLogger(); - global.mx_rage_logger.monkeyPatch(window.console); + setLogExtension(global.mx_rage_logger.log); return tryInitStorage(); } @@ -591,3 +560,42 @@ const getCircularReplacer = (): StringifyReplacer => { return value; }; }; + +enum LogLevel { + trace = 0, + debug = 1, + info = 2, + warn = 3, + error = 4, + silent = 5, +} + +type LogExtensionFunc = ( + level: LogLevel, + ...rest: (Error | DOMException | object | string)[] +) => void; +type LogLevelString = keyof typeof LogLevel; + +/** + * This method borrowed from livekit (who also ise loglevel and in turn essentially + * took loglevel's example honouring log levels). Adds a loglevel logging extension + * in the recommended way. + */ +export function setLogExtension(extension: LogExtensionFunc) { + const originalFactory = logger.methodFactory; + + logger.methodFactory = function (methodName, configLevel, loggerName) { + const rawMethod = originalFactory(methodName, configLevel, loggerName); + + const logLevel = LogLevel[methodName as LogLevelString]; + const needLog = logLevel >= configLevel && logLevel < LogLevel.silent; + + return (...args) => { + rawMethod.apply(this, args); + if (needLog) { + extension(logLevel, ...args); + } + }; + }; + logger.setLevel(logger.getLevel()); // Be sure to call setLevel method in order to apply plugin +} diff --git a/src/settings/submit-rageshake.ts b/src/settings/submit-rageshake.ts index 4103e35f..d871ed79 100644 --- a/src/settings/submit-rageshake.ts +++ b/src/settings/submit-rageshake.ts @@ -26,6 +26,7 @@ import { import pako from "pako"; import { MatrixEvent } from "matrix-js-sdk/src/models/event"; import { ClientEvent } from "matrix-js-sdk/src/client"; +import { logger } from "matrix-js-sdk/src/logger"; import { getLogsForReport } from "./rageshake"; import { useClient } from "../ClientContext"; @@ -296,7 +297,7 @@ export function useSubmitRageshake(): { setState({ sending: false, sent: true, error: undefined }); } catch (error) { setState({ sending: false, sent: false, error: error as Error }); - console.error(error); + logger.error(error); } }, [client, inspectorState, sending] diff --git a/src/video-grid/VideoGrid.tsx b/src/video-grid/VideoGrid.tsx index 8eb11a3d..7486656e 100644 --- a/src/video-grid/VideoGrid.tsx +++ b/src/video-grid/VideoGrid.tsx @@ -38,6 +38,7 @@ import { } from "@react-spring/web"; import useMeasure from "react-use-measure"; import { ResizeObserver as JuggleResizeObserver } from "@juggle/resize-observer"; +import { logger } from "matrix-js-sdk/src/logger"; import styles from "./VideoGrid.module.css"; import { Layout } from "../room/LayoutToggle"; @@ -298,7 +299,7 @@ function getFreedomLayoutTilePositions( } if (tileCount > 12) { - console.warn("Over 12 tiles is not currently supported"); + logger.warn("Over 12 tiles is not currently supported"); } const { layoutDirection, itemGridRatio } = getGridLayout(