From ba6e7c03ec1f8947bc1c7b6214c7a9566524bf4b Mon Sep 17 00:00:00 2001 From: sarayourfriend Date: Fri, 29 Mar 2024 17:45:08 +1100 Subject: [PATCH] Always use Logger class and try to log once per event This unifies logging across the backend to always use the Logger class, always only logs to stdout (rather than an inconsistent mix of stdout and stderr, depending on whether console.error was used), and removes logging where two log events happened for the same message For example, this pattern: ```js Logger.error("Whoops, something went wrong:") console.error(err) ``` That causes two separate log events, and depending on the log transport used, could cause relevant log messages to get split across multiple events and therefore be harder (usually just more tedious) to connect and debug in production environments. --- src/backend/middlewares/GalleryMWs.ts | 3 ++- src/backend/middlewares/RenderingMWs.ts | 5 ++++- src/backend/model/database/GalleryManager.ts | 2 +- src/backend/model/database/IndexingManager.ts | 4 ++-- src/backend/model/database/SQLConnection.ts | 3 +-- .../model/extension/ExtensionConfigWrapper.ts | 12 +++++------- src/backend/model/fileaccess/DiskManager.ts | 4 ++-- src/backend/model/fileaccess/MetadataLoader.ts | 11 ++++------- src/backend/model/fileaccess/TaskExecuter.ts | 3 ++- src/backend/model/jobs/JobProgressManager.ts | 5 +++-- src/backend/model/jobs/jobs/IndexingJob.ts | 3 +-- src/backend/model/jobs/jobs/Job.ts | 2 +- src/backend/model/messenger/StdoutMessenger.ts | 3 ++- src/backend/routes/ErrorRouter.ts | 3 +-- src/backend/server.ts | 2 +- 15 files changed, 32 insertions(+), 33 deletions(-) diff --git a/src/backend/middlewares/GalleryMWs.ts b/src/backend/middlewares/GalleryMWs.ts index da28a013..0b7dc1ec 100644 --- a/src/backend/middlewares/GalleryMWs.ts +++ b/src/backend/middlewares/GalleryMWs.ts @@ -17,6 +17,7 @@ import {LocationLookupException} from '../exceptions/LocationLookupException'; import {SupportedFormats} from '../../common/SupportedFormats'; import {ServerTime} from './ServerTimingMWs'; import {SortByTypes} from '../../common/entities/SortingMethods'; +import {Logger} from '../Logger'; export class GalleryMWs { @ServerTime('1.db', 'List Directory') @@ -109,7 +110,7 @@ export class GalleryMWs { }); res.on('close', () => { - console.log('zip ' + archive.pointer() + ' bytes'); + Logger.info('zip ' + archive.pointer() + ' bytes'); }); archive.on('error', (err: Error) => { diff --git a/src/backend/middlewares/RenderingMWs.ts b/src/backend/middlewares/RenderingMWs.ts index bb4ca0c1..a0f4b397 100644 --- a/src/backend/middlewares/RenderingMWs.ts +++ b/src/backend/middlewares/RenderingMWs.ts @@ -132,11 +132,14 @@ export class RenderingMWs { ): void { if (err instanceof ErrorDTO) { if (err.details) { + const logFn = Logger.logLevelForError(err.code) + + logFn('Handled error:'); LoggerRouter.log(logFn, req, res); // use separate rendering for detailsStr const d = err.detailsStr; delete err.detailsStr; - console.log(err); + logFn(err); err.detailsStr = d; delete err.details; // do not send back error object to the client side diff --git a/src/backend/model/database/GalleryManager.ts b/src/backend/model/database/GalleryManager.ts index 7d551e1e..06995371 100644 --- a/src/backend/model/database/GalleryManager.ts +++ b/src/backend/model/database/GalleryManager.ts @@ -123,7 +123,7 @@ export class GalleryManager { ); ObjectManagers.getInstance() .IndexingManager.indexDirectory(relativeDirectoryName) - .catch(console.error); + .catch(Logger.error); } return await this.getParentDirFromId(connection, dir.id); } diff --git a/src/backend/model/database/IndexingManager.ts b/src/backend/model/database/IndexingManager.ts index a07bd083..fd09e679 100644 --- a/src/backend/model/database/IndexingManager.ts +++ b/src/backend/model/database/IndexingManager.ts @@ -100,13 +100,13 @@ export class IndexingManager { resolve(dirClone); // save directory to DB - this.queueForSave(scannedDirectory).catch(console.error); + this.queueForSave(scannedDirectory).catch(Logger.error); } catch (error) { NotificationManager.warning( 'Unknown indexing error for: ' + relativeDirectoryName, error.toString() ); - console.error(error); + Logger.error(error); return reject(error); } }); diff --git a/src/backend/model/database/SQLConnection.ts b/src/backend/model/database/SQLConnection.ts index 07eb2ec9..047ef7a3 100644 --- a/src/backend/model/database/SQLConnection.ts +++ b/src/backend/model/database/SQLConnection.ts @@ -155,8 +155,7 @@ export class SQLConnection { this.connection = null; } } catch (err) { - console.error('Error during closing sql db:'); - console.error(err); + Logger.error('Error during closing sql db:\n', err); } } diff --git a/src/backend/model/extension/ExtensionConfigWrapper.ts b/src/backend/model/extension/ExtensionConfigWrapper.ts index eaa7270f..5f5f248a 100644 --- a/src/backend/model/extension/ExtensionConfigWrapper.ts +++ b/src/backend/model/extension/ExtensionConfigWrapper.ts @@ -3,7 +3,7 @@ import {PrivateConfigClass} from '../../../common/config/private/PrivateConfigCl import {ConfigClassBuilder} from 'typeconfig/node'; import {ExtensionConfigTemplateLoader} from './ExtensionConfigTemplateLoader'; import {NotificationManager} from '../NotifocationManager'; - +import {Logger} from '../../Logger'; const LOG_TAG = '[ExtensionConfigWrapper]'; @@ -19,10 +19,9 @@ export class ExtensionConfigWrapper { await pc.load(); // loading the basic configs, but we do not know the extension config hierarchy yet } catch (e) { - console.error(LOG_TAG, 'Error during loading config. Reverting to defaults.'); - console.error(e); + Logger.error(LOG_TAG, 'Error during loading config. Reverting to defaults.\n', e); if (showDetailedError) { - console.error(LOG_TAG, 'This is most likely due to: 1) you added a bad configuration in the server.json OR 2) The configuration changed in the latest release.'); + Logger.error(LOG_TAG, 'This is most likely due to: 1) you added a bad configuration in the server.json OR 2) The configuration changed in the latest release.'); NotificationManager.error('Can\'t load config. Reverting to default. This is most likely due to: 1) you added a bad configuration in the server.json OR 2) The configuration changed in the latest release.', (e.toString ? e.toString() : JSON.stringify(e))); } } @@ -37,10 +36,9 @@ export class ExtensionConfigWrapper { pc.loadSync(); // loading the basic configs, but we do not know the extension config hierarchy yet } catch (e) { - console.error(LOG_TAG, 'Error during loading config. Reverting to defaults.'); - console.error(e); + Logger.error(LOG_TAG, 'Error during loading config. Reverting to defaults.\n', e); if (showDetailedError) { - console.error(LOG_TAG, 'This is most likely due to: 1) you added a bad configuration in the server.json OR 2) The configuration changed in the latest release.'); + Logger.error(LOG_TAG, 'This is most likely due to: 1) you added a bad configuration in the server.json OR 2) The configuration changed in the latest release.'); NotificationManager.error('Ca\'nt load config. Reverting to default. This is most likely due to: 1) you added a bad configuration in the server.json OR 2) The configuration changed in the latest release.', (e.toString ? e.toString() : JSON.stringify(e))); } } diff --git a/src/backend/model/fileaccess/DiskManager.ts b/src/backend/model/fileaccess/DiskManager.ts index e07ff2e3..5efaf424 100644 --- a/src/backend/model/fileaccess/DiskManager.ts +++ b/src/backend/model/fileaccess/DiskManager.ts @@ -179,7 +179,7 @@ export class DiskManager { 'Unknown directory reading error, skipping: ' + path.join(relativeDirectoryName, file), err.toString() ); - console.error(err); + Logger.error(err); } } else if (PhotoProcessing.isPhoto(fullFilePath)) { try { @@ -218,7 +218,7 @@ export class DiskManager { ', reason: ' + err.toString() ); - console.error(err); + Logger.error(err); } } else if (VideoProcessing.isVideo(fullFilePath)) { try { diff --git a/src/backend/model/fileaccess/MetadataLoader.ts b/src/backend/model/fileaccess/MetadataLoader.ts index 5a61af21..84ffa2be 100644 --- a/src/backend/model/fileaccess/MetadataLoader.ts +++ b/src/backend/model/fileaccess/MetadataLoader.ts @@ -41,7 +41,7 @@ export class MetadataLoader { metadata.fileSize = stat.size; metadata.creationDate = stat.mtime.getTime(); //Default date is file system time of last modification } catch (err) { - console.log(err); + Logger.info(err); // ignoring errors } try { @@ -214,8 +214,7 @@ export class MetadataLoader { try { await fileHandle.read(data, 0, bufferSize, 0); } catch (err) { - Logger.error(LOG_TAG, 'Error during reading photo: ' + fullPath); - console.error(err); + Logger.error(LOG_TAG, 'Error during reading photo: ' + fullPath + '\n', err); return MetadataLoader.EMPTY_METADATA; } finally { await fileHandle.close(); @@ -295,13 +294,11 @@ export class MetadataLoader { metadata.creationDate = 0; } } catch (err) { - Logger.error(LOG_TAG, 'Error during reading photo: ' + fullPath); - console.error(err); + Logger.error(LOG_TAG, 'Error during reading photo: ' + fullPath + '\n', err); return MetadataLoader.EMPTY_METADATA; } } catch (err) { - Logger.error(LOG_TAG, 'Error during reading photo: ' + fullPath); - console.error(err); + Logger.error(LOG_TAG, 'Error during reading photo: ' + fullPath + '\n', err); return MetadataLoader.EMPTY_METADATA; } return metadata; diff --git a/src/backend/model/fileaccess/TaskExecuter.ts b/src/backend/model/fileaccess/TaskExecuter.ts index 4e203f03..33ee75f3 100644 --- a/src/backend/model/fileaccess/TaskExecuter.ts +++ b/src/backend/model/fileaccess/TaskExecuter.ts @@ -1,5 +1,6 @@ import {TaskQue} from './TaskQue'; import {EventLoopHandler} from '../EventLoopHandler'; +import { Logger } from '../../Logger'; export interface ITaskExecuter { execute(input: I): Promise; @@ -30,7 +31,7 @@ export class TaskExecuter implements ITaskExecuter { execute(input: I): Promise { const promise = this.taskQue.add(input).promise.obj; - this.run().catch(console.error); + this.run().catch(Logger.error); return promise; } } diff --git a/src/backend/model/jobs/JobProgressManager.ts b/src/backend/model/jobs/JobProgressManager.ts index a1c46426..4c637a2d 100644 --- a/src/backend/model/jobs/JobProgressManager.ts +++ b/src/backend/model/jobs/JobProgressManager.ts @@ -3,6 +3,7 @@ import * as path from 'path'; import {ProjectPath} from '../../ProjectPath'; import {Config} from '../../../common/config/private/Config'; import {JobProgressDTO, JobProgressStates,} from '../../../common/entities/job/JobProgressDTO'; +import { Logger } from '../../Logger'; export class JobProgressManager { private static readonly VERSION = 3; @@ -20,7 +21,7 @@ export class JobProgressManager { constructor() { this.dbPath = path.join(ProjectPath.DBFolder, 'jobs.db'); - this.loadDB().catch(console.error); + this.loadDB().catch(Logger.error); } get Progresses(): { [key: string]: JobProgressDTO } { @@ -89,7 +90,7 @@ export class JobProgressManager { return; } this.timer = setTimeout(async (): Promise => { - this.saveDB().catch(console.error); + this.saveDB().catch(Logger.error); this.timer = null; }, 5000); } diff --git a/src/backend/model/jobs/jobs/IndexingJob.ts b/src/backend/model/jobs/jobs/IndexingJob.ts index 65973e85..b85beb40 100644 --- a/src/backend/model/jobs/jobs/IndexingJob.ts +++ b/src/backend/model/jobs/jobs/IndexingJob.ts @@ -93,8 +93,7 @@ export class IndexingJob< } catch (e) { this.Progress.log('Skipping. Indexing failed for: ' + directory); this.Progress.Skipped++; - Logger.warn(LOG_TAG, 'Skipping. Indexing failed for: ' + directory); - console.error(e); + Logger.warn(LOG_TAG, 'Skipping. Indexing failed for: ' + directory, + '\n', e); } if (this.Progress.State !== JobProgressStates.running) { return false; diff --git a/src/backend/model/jobs/jobs/Job.ts b/src/backend/model/jobs/jobs/Job.ts index 36a7a0cb..ddf7c3f3 100644 --- a/src/backend/model/jobs/jobs/Job.ts +++ b/src/backend/model/jobs/jobs/Job.ts @@ -71,7 +71,7 @@ export abstract class Job = Record((resolve): void => { this.prResolve = resolve; }); - this.init().catch(console.error); + this.init().catch(Logger.error); this.run(); if (!this.IsInstant) { // if instant, wait for execution, otherwise, return right away diff --git a/src/backend/model/messenger/StdoutMessenger.ts b/src/backend/model/messenger/StdoutMessenger.ts index 1edbf20a..86cabbb6 100644 --- a/src/backend/model/messenger/StdoutMessenger.ts +++ b/src/backend/model/messenger/StdoutMessenger.ts @@ -1,6 +1,7 @@ import {MediaDTOWithThPath, Messenger} from './Messenger'; import {DynamicConfig} from '../../../common/entities/DynamicConfig'; import {DefaultMessengers} from '../../../common/entities/job/JobDTO'; +import { Logger } from '../../Logger'; export class StdoutMessenger extends Messenger { public readonly Name = DefaultMessengers[DefaultMessengers.Stdout]; @@ -12,6 +13,6 @@ export class StdoutMessenger extends Messenger { protected async sendMedia(config: never, media: MediaDTOWithThPath[]) { - console.log(media.map(m => m.thumbnailPath)); + Logger.info(media.map(m => m.thumbnailPath)); } } diff --git a/src/backend/routes/ErrorRouter.ts b/src/backend/routes/ErrorRouter.ts index 3507e631..6fc76e96 100644 --- a/src/backend/routes/ErrorRouter.ts +++ b/src/backend/routes/ErrorRouter.ts @@ -38,8 +38,7 @@ export class ErrorRouter { } // Flush out the stack to the console - Logger.error('Unexpected error:'); - console.error(err); + Logger.error('Unexpected error:\n', err); return next( new ErrorDTO( ErrorCodes.SERVER_ERROR, diff --git a/src/backend/server.ts b/src/backend/server.ts index 382330e6..9c0a9213 100644 --- a/src/backend/server.ts +++ b/src/backend/server.ts @@ -51,7 +51,7 @@ export class Server { 'Running in DEBUG mode, set env variable NODE_ENV=production to disable ' ); } - this.init(listen).catch(console.error); + this.init(listen).catch(Logger.error); } get Server(): HttpServer {