From 11477a11851749000115efd5bd4be85f6516bee2 Mon Sep 17 00:00:00 2001 From: Tom Moor Date: Mon, 25 Apr 2022 23:31:30 -0700 Subject: [PATCH] chore: Centralize clientside logging --- app/components/CollectionIcon.tsx | 5 +- app/components/ErrorBoundary.tsx | 7 +- app/editor/index.tsx | 3 +- app/hooks/usePersistedState.ts | 3 +- app/index.tsx | 9 +- .../Document/components/MultiplayerEditor.tsx | 15 +++- app/scenes/Search/Search.tsx | 5 +- app/utils/files.ts | 11 ++- app/utils/logger.ts | 86 +++++++++++++++++++ server/logging/logger.ts | 6 +- 10 files changed, 128 insertions(+), 22 deletions(-) create mode 100644 app/utils/logger.ts diff --git a/app/components/CollectionIcon.tsx b/app/components/CollectionIcon.tsx index 427e55491..f01551548 100644 --- a/app/components/CollectionIcon.tsx +++ b/app/components/CollectionIcon.tsx @@ -5,6 +5,7 @@ import * as React from "react"; import Collection from "~/models/Collection"; import { icons } from "~/components/IconPicker"; import useStores from "~/hooks/useStores"; +import Logger from "~/utils/logger"; type Props = { collection: Collection; @@ -36,7 +37,9 @@ function ResolvedCollectionIcon({ const Component = icons[collection.icon].component; return ; } catch (error) { - console.warn("Failed to render custom icon " + collection.icon); + Logger.warn("Failed to render custom icon", { + icon: collection.icon, + }); } } diff --git a/app/components/ErrorBoundary.tsx b/app/components/ErrorBoundary.tsx index 83749e0f3..6445c4fa9 100644 --- a/app/components/ErrorBoundary.tsx +++ b/app/components/ErrorBoundary.tsx @@ -1,4 +1,3 @@ -import * as Sentry from "@sentry/react"; import { observable } from "mobx"; import { observer } from "mobx-react"; import * as React from "react"; @@ -11,6 +10,7 @@ import PageTitle from "~/components/PageTitle"; import Text from "~/components/Text"; import env from "~/env"; import isHosted from "~/utils/isHosted"; +import Logger from "~/utils/logger"; type Props = WithTranslation & { reloadOnChunkMissing?: boolean; @@ -26,7 +26,6 @@ class ErrorBoundary extends React.Component { componentDidCatch(error: Error) { this.error = error; - console.error(error); if ( this.props.reloadOnChunkMissing && @@ -40,9 +39,7 @@ class ErrorBoundary extends React.Component { return; } - if (env.SENTRY_DSN) { - Sentry.captureException(error); - } + Logger.error("ErrorBoundary", error); } handleReload = () => { diff --git a/app/editor/index.tsx b/app/editor/index.tsx index 21cfc9d73..6d7b64cb8 100644 --- a/app/editor/index.tsx +++ b/app/editor/index.tsx @@ -28,6 +28,7 @@ import { EmbedDescriptor, EventType } from "@shared/editor/types"; import EventEmitter from "@shared/utils/events"; import Flex from "~/components/Flex"; import { Dictionary } from "~/hooks/useDictionary"; +import Logger from "~/utils/logger"; import BlockMenu from "./components/BlockMenu"; import ComponentView from "./components/ComponentView"; import EditorContext from "./components/EditorContext"; @@ -476,7 +477,7 @@ export class Editor extends React.PureComponent< // querySelector will throw an error if the hash begins with a number // or contains a period. This is protected against now by safeSlugify // however previous links may be in the wild. - console.warn(`Attempted to scroll to invalid hash: ${hash}`, err); + Logger.debug("editor", `Attempted to scroll to invalid hash: ${hash}`); } } diff --git a/app/hooks/usePersistedState.ts b/app/hooks/usePersistedState.ts index 1087a715f..f79c9061a 100644 --- a/app/hooks/usePersistedState.ts +++ b/app/hooks/usePersistedState.ts @@ -1,6 +1,7 @@ import * as React from "react"; import { Primitive } from "utility-types"; import Storage from "~/utils/Storage"; +import Logger from "~/utils/logger"; import useEventListener from "./useEventListener"; /** @@ -32,7 +33,7 @@ export default function usePersistedState( Storage.set(key, valueToStore); } catch (error) { // A more advanced implementation would handle the error case - console.log(error); + Logger.debug("misc", "Failed to persist state", { error }); } }; diff --git a/app/index.tsx b/app/index.tsx index 9a65d2d40..b9681bf88 100644 --- a/app/index.tsx +++ b/app/index.tsx @@ -17,6 +17,7 @@ import Toasts from "~/components/Toasts"; import env from "~/env"; import Routes from "./routes"; import history from "./utils/history"; +import Logger from "./utils/logger"; import { initSentry } from "./utils/sentry"; initI18n(); @@ -40,10 +41,14 @@ if ("serviceWorker" in window.navigator) { if (maybePromise?.then) { maybePromise .then((registration) => { - console.log("SW registered: ", registration); + Logger.debug("lifecycle", "SW registered: ", registration); }) .catch((registrationError) => { - console.log("SW registration failed: ", registrationError); + Logger.debug( + "lifecycle", + "SW registration failed: ", + registrationError + ); }); } }); diff --git a/app/scenes/Document/components/MultiplayerEditor.tsx b/app/scenes/Document/components/MultiplayerEditor.tsx index 1ca743dbe..6b5bbcb8b 100644 --- a/app/scenes/Document/components/MultiplayerEditor.tsx +++ b/app/scenes/Document/components/MultiplayerEditor.tsx @@ -16,6 +16,7 @@ import useStores from "~/hooks/useStores"; import useToasts from "~/hooks/useToasts"; import MultiplayerExtension from "~/multiplayer/MultiplayerExtension"; import { supportsPassiveListener } from "~/utils/browser"; +import Logger from "~/utils/logger"; import { homePath } from "~/utils/routeHelpers"; type Props = EditorProps & { @@ -139,15 +140,21 @@ function MultiplayerEditor({ onSynced, ...props }: Props, ref: any) { if (debug) { provider.on("status", (ev: ConnectionStatusEvent) => - console.log("status", ev.status) + Logger.debug("collaboration", "status", ev) ); provider.on("message", (ev: MessageEvent) => - console.log("incoming", ev.message) + Logger.debug("collaboration", "incoming", { + message: ev.message, + }) ); provider.on("outgoingMessage", (ev: MessageEvent) => - console.log("outgoing", ev.message) + Logger.debug("collaboration", "outgoing", { + message: ev.message, + }) + ); + localProvider.on("synced", () => + Logger.debug("collaboration", "local synced") ); - localProvider.on("synced", () => console.log("local synced")); } provider.on("status", (ev: ConnectionStatusEvent) => diff --git a/app/scenes/Search/Search.tsx b/app/scenes/Search/Search.tsx index fecaec684..e75182763 100644 --- a/app/scenes/Search/Search.tsx +++ b/app/scenes/Search/Search.tsx @@ -23,6 +23,7 @@ import RegisterKeyDown from "~/components/RegisterKeyDown"; import Scene from "~/components/Scene"; import Text from "~/components/Text"; import withStores from "~/components/withStores"; +import Logger from "~/utils/logger"; import { searchPath } from "~/utils/routeHelpers"; import { decodeURIComponentSafe } from "~/utils/urls"; import CollectionFilter from "./components/CollectionFilter"; @@ -257,9 +258,9 @@ class Search extends React.Component { } else { this.offset += DEFAULT_PAGINATION_LIMIT; } - } catch (err) { + } catch (error) { + Logger.error("Search query failed", error); this.lastQuery = ""; - throw err; } finally { this.isLoading = false; } diff --git a/app/utils/files.ts b/app/utils/files.ts index 8cb7b1b7d..62f0274b2 100644 --- a/app/utils/files.ts +++ b/app/utils/files.ts @@ -1,6 +1,6 @@ -import * as Sentry from "@sentry/react"; import invariant from "invariant"; import { client } from "./ApiClient"; +import Logger from "./logger"; type UploadOptions = { /** The user facing name of the file */ @@ -45,7 +45,6 @@ export const uploadFile = async ( } // Using XMLHttpRequest instead of fetch because fetch doesn't support progress - let error; const xhr = new XMLHttpRequest(); const success = await new Promise((resolve) => { xhr.upload.addEventListener("progress", (event) => { @@ -53,7 +52,12 @@ export const uploadFile = async ( options.onProgress(event.loaded / event.total); } }); - xhr.addEventListener("error", (err) => (error = err)); + xhr.addEventListener("error", () => { + Logger.error( + "File upload failed", + new Error(`${xhr.status} ${xhr.statusText}`) + ); + }); xhr.addEventListener("loadend", () => { resolve(xhr.readyState === 4 && xhr.status >= 200 && xhr.status < 400); }); @@ -62,7 +66,6 @@ export const uploadFile = async ( }); if (!success) { - Sentry.captureException(error); throw new Error("Upload failed"); } diff --git a/app/utils/logger.ts b/app/utils/logger.ts new file mode 100644 index 000000000..1fdb9683e --- /dev/null +++ b/app/utils/logger.ts @@ -0,0 +1,86 @@ +import * as Sentry from "@sentry/react"; +import env from "~/env"; + +type LogCategory = + | "lifecycle" + | "http" + | "editor" + | "router" + | "collaboration" + | "misc"; + +type Extra = Record; + +class Logger { + /** + * Log information + * + * @param category A log message category that will be prepended + * @param extra Arbitrary data to be logged that will appear in prod logs + */ + info(label: LogCategory, message: string, extra?: Extra) { + console.info(`[${label}] ${message}`, extra); + } + + /** + * Debug information + * + * @param category A log message category that will be prepended + * @param extra Arbitrary data to be logged + */ + debug(label: LogCategory, message: string, extra?: Extra) { + if (env.ENVIRONMENT === "development") { + console.debug(`[${label}] ${message}`, extra); + } + } + + /** + * Log a warning + * + * @param message A warning message + * @param extra Arbitrary data to be logged that will appear in prod logs + */ + warn(message: string, extra?: Extra) { + if (env.SENTRY_DSN) { + Sentry.withScope(function (scope) { + scope.setLevel(Sentry.Severity.Warning); + + for (const key in extra) { + scope.setExtra(key, extra[key]); + } + + Sentry.captureMessage(message); + }); + } + + console.warn(message, extra); + } + + /** + * Report a runtime error + * + * @param message A description of the error + * @param error The error that occurred + * @param extra Arbitrary data to be logged that will appear in prod logs + */ + error(message: string, error: Error, extra?: Extra) { + if (env.SENTRY_DSN) { + Sentry.withScope(function (scope) { + scope.setLevel(Sentry.Severity.Error); + + for (const key in extra) { + scope.setExtra(key, extra[key]); + } + + Sentry.captureException(error); + }); + } + + console.error(message, { + error, + extra, + }); + } +} + +export default new Logger(); diff --git a/server/logging/logger.ts b/server/logging/logger.ts index 44d0c1dcc..b07bd1ae4 100644 --- a/server/logging/logger.ts +++ b/server/logging/logger.ts @@ -74,9 +74,10 @@ class Logger { if (process.env.SENTRY_DSN) { Sentry.withScope(function (scope) { + scope.setLevel(Sentry.Severity.Warning); + for (const key in extra) { scope.setExtra(key, extra[key]); - scope.setLevel(Sentry.Severity.Warning); } Sentry.captureMessage(message); @@ -105,9 +106,10 @@ class Logger { if (process.env.SENTRY_DSN) { Sentry.withScope(function (scope) { + scope.setLevel(Sentry.Severity.Error); + for (const key in extra) { scope.setExtra(key, extra[key]); - scope.setLevel(Sentry.Severity.Error); } Sentry.captureException(error);