From c9e5ee9ddb9c7ef243f35e5efcc92f678487260a Mon Sep 17 00:00:00 2001 From: Abdullah Atta Date: Thu, 18 Apr 2024 09:37:21 +0500 Subject: [PATCH] core: use database logger instead of console.error --- packages/core/src/api/index.ts | 2 +- packages/core/src/api/monographs.ts | 3 ++- packages/core/src/api/sync/index.ts | 4 ++-- packages/core/src/api/token-manager.ts | 2 +- packages/core/src/api/user-manager.ts | 9 ++++---- packages/core/src/api/vault.ts | 3 ++- packages/core/src/collections/attachments.ts | 9 ++++---- packages/core/src/content-types/tiptap.ts | 5 +++- packages/core/src/database/backup.ts | 9 ++++++-- packages/core/src/database/fs.ts | 3 ++- packages/core/src/database/index.ts | 24 ++++++++++++-------- packages/core/src/utils/http.ts | 2 +- packages/logger/src/index.ts | 20 +++++++++------- 13 files changed, 59 insertions(+), 36 deletions(-) diff --git a/packages/core/src/api/index.ts b/packages/core/src/api/index.ts index 4130c12de..03c9a5044 100644 --- a/packages/core/src/api/index.ts +++ b/packages/core/src/api/index.ts @@ -311,7 +311,7 @@ class Database { // we must not wait on network requests that's why // no await - this.monographs.refresh().catch(console.error); + this.monographs.refresh().catch(logger.error); } disconnectSSE() { diff --git a/packages/core/src/api/monographs.ts b/packages/core/src/api/monographs.ts index 8344225fd..0980af642 100644 --- a/packages/core/src/api/monographs.ts +++ b/packages/core/src/api/monographs.ts @@ -23,6 +23,7 @@ import Database from "."; import { Note, isDeleted } from "../types"; import { Cipher } from "@notesnook/crypto"; import { isFalse } from "../database"; +import { logger } from "../logger"; type BaseMonograph = { id: string; @@ -61,7 +62,7 @@ export class Monographs { await this.db.kv().write("monographs", monographs); if (monographs) this.monographs = monographs; } catch (e) { - console.error(e); + logger.error(e, "Error while refreshing monographs."); } } diff --git a/packages/core/src/api/sync/index.ts b/packages/core/src/api/sync/index.ts index ae6637e5c..093799ea5 100644 --- a/packages/core/src/api/sync/index.ts +++ b/packages/core/src/api/sync/index.ts @@ -267,7 +267,7 @@ class Sync { async stop() { // refresh monographs - await this.db.monographs.refresh().catch(console.error); + await this.db.monographs.refresh().catch(this.logger.error); // update trash cache await this.db.trash.buildCache(); @@ -412,7 +412,7 @@ class Sync { await promiseTimeout(30000, this.connection.start()); } } catch (e) { - console.error(e); + this.logger.error(e, "Could not connect to the Sync server."); if (e instanceof Error) { this.logger.warn(e.message); throw new Error( diff --git a/packages/core/src/api/token-manager.ts b/packages/core/src/api/token-manager.ts index 67eefc2b5..599fcdc60 100644 --- a/packages/core/src/api/token-manager.ts +++ b/packages/core/src/api/token-manager.ts @@ -154,7 +154,7 @@ async function getSafeToken(action: () => Promise, errorMessage: string) { try { return await action(); } catch (e) { - console.error(errorMessage, e); + logger.error(e, errorMessage); if ( e instanceof Error && (e.message === "invalid_grant" || e.message === "invalid_client") diff --git a/packages/core/src/api/user-manager.ts b/packages/core/src/api/user-manager.ts index effeb544c..125a7f4d1 100644 --- a/packages/core/src/api/user-manager.ts +++ b/packages/core/src/api/user-manager.ts @@ -25,6 +25,7 @@ import { EV, EVENTS } from "../common"; import { HealthCheck } from "./healthcheck"; import Database from "."; import { SerializedKey } from "@notesnook/crypto"; +import { logger } from "../logger"; const ENDPOINTS = { signup: "/users", @@ -241,7 +242,7 @@ class UserManager { await this.db.syncer.devices.unregister(); if (revoke) await this.tokenManager.revokeToken(); } catch (e) { - console.error(e); + logger.error(e, "Error logging out user.", { revoke, reason }); } finally { await this.db.reset(); EV.publish(EVENTS.userLoggedOut, reason); @@ -319,7 +320,7 @@ class UserManager { return await this.getUser(); } } catch (e) { - console.error("Error fetching user", e); + logger.error(e, "Error fetching user"); return await this.getUser(); } } @@ -400,10 +401,10 @@ class UserManager { if (!plainData) return; return JSON.parse(plainData); } catch (e) { - console.error(e); + logger.error(e, "Could not get attachments encryption key."); if (e instanceof Error) throw new Error( - `Could not get attachments encryption key. Please make sure you have Internet access. Error: ${e.message}` + `Could not get attachments encryption key. Error: ${e.message}` ); } } diff --git a/packages/core/src/api/vault.ts b/packages/core/src/api/vault.ts index 05111c342..f3ecd58dc 100644 --- a/packages/core/src/api/vault.ts +++ b/packages/core/src/api/vault.ts @@ -23,6 +23,7 @@ import { CHECK_IDS, EV, EVENTS, checkIsUserPremium } from "../common"; import { isCipher } from "../database/crypto"; import { NoteContent } from "../collections/session-content"; import { Note } from "../types"; +import { logger } from "../logger"; export const VAULT_ERRORS = { noVault: "ERR_NO_VAULT", @@ -119,7 +120,7 @@ export default class Vault { `${Date.now()}` ); } catch (e) { - console.error(e); + logger.error(e, `Could not decrypt content of note ${noteId}`); throw new Error( `Could not decrypt content of note ${noteId}. Error: ${ (e as Error).message diff --git a/packages/core/src/collections/attachments.ts b/packages/core/src/collections/attachments.ts index dbd15de85..2786c7a91 100644 --- a/packages/core/src/collections/attachments.ts +++ b/packages/core/src/collections/attachments.ts @@ -30,6 +30,7 @@ import Database from "../api"; import { FilteredSelector, SQLCollection } from "../database/sql-collection"; import { isFalse } from "../database"; import { sql } from "kysely"; +import { logger } from "../logger"; export class Attachments implements ICollection { name = "attachments"; @@ -108,7 +109,7 @@ export class Attachments implements ICollection { } > ) { - if (!item) return console.error("attachment cannot be undefined"); + if (!item) throw new Error("attachment cannot be undefined"); if (!item.hash) throw new Error("Please provide attachment hash."); const oldAttachment = await this.attachment(item.hash); @@ -148,11 +149,11 @@ export class Attachments implements ICollection { !chunkSize || !key ) { - console.error( + logger.error( + {}, "Attachment is invalid because all properties are required:", - attachment + { attachment } ); - // throw new Error("Could not add attachment: all properties are required."); return; } diff --git a/packages/core/src/content-types/tiptap.ts b/packages/core/src/content-types/tiptap.ts index 9c1d9dcd8..f38f0109c 100644 --- a/packages/core/src/content-types/tiptap.ts +++ b/packages/core/src/content-types/tiptap.ts @@ -42,6 +42,7 @@ import { } from "../utils/internal-link"; import { Element } from "domhandler"; import { render } from "dom-serializer"; +import { logger } from "../logger"; export type ResolveHashes = ( hashes: string[] @@ -322,7 +323,9 @@ export class Tiptap { images[image.id] = hash; } catch (e) { - console.error(e); + logger.error(e, "Failed to save image attachment.", { + filename: image.filename + }); images[image.id] = false; } } diff --git a/packages/core/src/database/backup.ts b/packages/core/src/database/backup.ts index b0ec9396e..9f0612fe1 100644 --- a/packages/core/src/database/backup.ts +++ b/packages/core/src/database/backup.ts @@ -35,6 +35,7 @@ import { migrateItem } from "../migrations"; import { DatabaseCollection } from "./index.js"; import { DefaultColors } from "../collections/colors.js"; import { toChunks } from "../utils/array.js"; +import { logger } from "../logger.js"; type BackupDataItem = MaybeDeletedItem | string[]; type BackupPlatform = "web" | "mobile" | "node"; @@ -340,7 +341,11 @@ export default class Backup { }; } - async import(backup: LegacyBackupFile | BackupFile, password?: string, encryptionKey?: string) { + async import( + backup: LegacyBackupFile | BackupFile, + password?: string, + encryptionKey?: string + ) { if (!this.validate(backup)) throw new Error("Invalid backup."); backup = this.migrateBackup(backup); @@ -364,7 +369,7 @@ export default class Backup { try { decryptedData = await this.db.storage().decrypt(key, backup.data); } catch (e) { - console.error(e); + logger.error(e, "Failed to import backup"); if (e instanceof Error) { if ( e.message.includes("ciphertext cannot be decrypted") || diff --git a/packages/core/src/database/fs.ts b/packages/core/src/database/fs.ts index d7bea463f..b0e41076e 100644 --- a/packages/core/src/database/fs.ts +++ b/packages/core/src/database/fs.ts @@ -25,6 +25,7 @@ import { } from "../interfaces"; import { DataFormat, SerializedKey } from "@notesnook/crypto/dist/src/types"; import { EV, EVENTS } from "../common"; +import { logger } from "../logger"; export type FileStorageAccessor = () => FileStorage; export type DownloadableFile = { @@ -121,7 +122,7 @@ export class FileStorage { let error = null; const result = await execute().catch((e) => { - console.error("Failed to upload attachment:", e); + logger.error(e, "failed to upload attachment", { hash: filename }); error = e; return false; }); diff --git a/packages/core/src/database/index.ts b/packages/core/src/database/index.ts index 59455ef39..b0d63ee02 100644 --- a/packages/core/src/database/index.ts +++ b/packages/core/src/database/index.ts @@ -60,6 +60,7 @@ import { } from "../types"; import { NNMigrationProvider } from "./migrations"; import { createTriggers } from "./triggers"; +import { logger } from "../logger"; // type FilteredKeys = { // [P in keyof T]: T[P] extends U ? P : never; @@ -300,21 +301,22 @@ export async function initializeDatabase(db: Kysely) { }); const { error, results } = await migrator.migrateToLatest(); - results?.forEach((it) => { - if (it.status === "Error") - console.error(`failed to execute migration "${it.migrationName}"`); - }); + if (error) + throw error instanceof Error ? error : new Error(JSON.stringify(error)); - if (error) { - console.error("failed to run `migrateToLatest`"); - console.error(error); - } + const errors = results?.filter((it) => it.status === "Error") || []; + if (errors.length > 0) + throw new Error( + `failed to execute migrations: ${errors + .map((e) => e.migrationName) + .join(", ")}` + ); await createTriggers(db); return db; } catch (e) { - console.error(e); + logger.error(e, "Failed to initialized database."); await db.destroy(); throw e; } @@ -334,6 +336,10 @@ export type SQLiteOptions = { }; export async function createDatabase(name: string, options: SQLiteOptions) { const db = new Kysely({ + log: (event) => { + if (event.queryDurationMillis > 5) + console.warn(event.query.sql, event.queryDurationMillis); + }, dialect: options.dialect(name, async () => { await db.connection().execute(async (db) => { await setupDatabase(db, options); diff --git a/packages/core/src/utils/http.ts b/packages/core/src/utils/http.ts index ce256919c..bb2f78f17 100644 --- a/packages/core/src/utils/http.ts +++ b/packages/core/src/utils/http.ts @@ -180,7 +180,7 @@ async function handleResponse(response: Response) { ); } } catch (e) { - logger.error(e as Error, "Error while sending request:", { + logger.error(e, "Error while sending request:", { url: response.url }); throw e; diff --git a/packages/logger/src/index.ts b/packages/logger/src/index.ts index 9114863c6..9763e38af 100644 --- a/packages/logger/src/index.ts +++ b/packages/logger/src/index.ts @@ -22,7 +22,7 @@ import { ILogReporter, LoggerConfig, LogLevel } from "./types"; type LogLevelFunc = (message: string, extras?: Record) => void; type ErrorLogLevelFunc = ( - error: Error, + error: Error | unknown, fallbackMessage?: string, extras?: Record ) => void; @@ -101,20 +101,24 @@ function logLevelFactory(level: LogLevel, config: LoggerConfig) { function errorLogLevelFactory(level: LogLevel, config: LoggerConfig) { return ( - error: Error, + error: Error | unknown, fallbackMessage?: string, extras?: Record ) => { const now = Date.now(); config.reporter.write({ level, - message: error.stack - ? error.stack.trim() - : fallbackMessage - ? fallbackMessage - : "An error occurred.", + message: + error instanceof Error && error.stack + ? error.stack.trim() + : fallbackMessage + ? fallbackMessage + : "An unknown error occurred.", timestamp: now, - extras, + extras: + error instanceof Error + ? { ...extras, fallbackMessage } + : { ...extras, error }, scope: config.scope, elapsed: now - config.lastTime });