From eb3ce9be3084042da7556332cc5c40c3e7250d42 Mon Sep 17 00:00:00 2001 From: Cody Tseng <64680921+CodyTseng@users.noreply.github.com> Date: Thu, 25 Jan 2024 20:23:01 +0800 Subject: [PATCH] feat: improve Logger (#10) --- .eslintrc.js | 78 ++++++++-------- .../services/console-logger.service.spec.ts | 93 +++++++++++++++++-- packages/common/src/enums/index.ts | 1 + packages/common/src/enums/log.enum.ts | 6 ++ ...rface.ts => event-repository.interface.ts} | 0 packages/common/src/interfaces/index.ts | 2 +- .../common/src/interfaces/logger.interface.ts | 8 +- .../src/services/console-logger.service.ts | 31 ++++++- .../__test__/services/event.service.spec.ts | 21 +++-- .../services/subscription.service.spec.ts | 15 +-- .../src/interfaces/handle-result.interface.ts | 6 +- packages/core/src/nostr-relay.ts | 16 +++- packages/core/src/services/event.service.ts | 14 ++- .../core/src/services/subscription.service.ts | 22 ++--- tsconfig.spec.json | 5 + 15 files changed, 231 insertions(+), 87 deletions(-) create mode 100644 packages/common/src/enums/log.enum.ts rename packages/common/src/interfaces/{event-respository.interface.ts => event-repository.interface.ts} (100%) create mode 100644 tsconfig.spec.json diff --git a/.eslintrc.js b/.eslintrc.js index b66c269b..aa793f0d 100644 --- a/.eslintrc.js +++ b/.eslintrc.js @@ -1,42 +1,42 @@ module.exports = { - root: true, - env: { - node: true, + root: true, + env: { + node: true, + }, + plugins: ['@typescript-eslint/eslint-plugin'], + extends: ['plugin:@typescript-eslint/recommended'], + overrides: [ + { + files: ['**/*.ts'], + parser: '@typescript-eslint/parser', + parserOptions: { + project: 'tsconfig.json', + sourceType: 'module', + }, + rules: { + '@typescript-eslint/interface-name-prefix': 'off', + '@typescript-eslint/explicit-function-return-type': 'warn', + '@typescript-eslint/no-explicit-any': 'off', + '@typescript-eslint/explicit-module-boundary-types': 'off', + '@typescript-eslint/ban-types': 'off', + '@typescript-eslint/no-array-constructor': 'off', + }, }, - plugins: ['@typescript-eslint/eslint-plugin'], - extends: ['plugin:@typescript-eslint/recommended'], - overrides: [ - { - files: ['**/*.ts'], - parser: '@typescript-eslint/parser', - parserOptions: { - project: 'tsconfig.json', - sourceType: 'module', - }, - rules: { - '@typescript-eslint/interface-name-prefix': 'off', - '@typescript-eslint/explicit-function-return-type': 'warn', - '@typescript-eslint/no-explicit-any': 'off', - '@typescript-eslint/explicit-module-boundary-types': 'off', - '@typescript-eslint/ban-types': 'off', - '@typescript-eslint/no-array-constructor': 'off', - }, + { + files: ['**/*.spec.ts', 'integration/**/*.ts'], + parser: '@typescript-eslint/parser', + parserOptions: { + project: 'tsconfig.spec.json', + sourceType: 'module', + }, + rules: { + '@typescript-eslint/interface-name-prefix': 'off', + '@typescript-eslint/explicit-function-return-type': 'off', + '@typescript-eslint/no-explicit-any': 'off', + '@typescript-eslint/explicit-module-boundary-types': 'off', + '@typescript-eslint/ban-types': 'off', + '@typescript-eslint/no-empty-function': 'off', }, - { - files: ['**/*.spec.ts', 'integration/**/*.ts'], - parser: '@typescript-eslint/parser', - parserOptions: { - project: 'tsconfig.json', - sourceType: 'module', - }, - rules: { - '@typescript-eslint/interface-name-prefix': 'off', - '@typescript-eslint/explicit-function-return-type': 'off', - '@typescript-eslint/no-explicit-any': 'off', - '@typescript-eslint/explicit-module-boundary-types': 'off', - '@typescript-eslint/ban-types': 'off', - '@typescript-eslint/no-empty-function': 'off', - }, - } - ] - }; \ No newline at end of file + } + ] +}; \ No newline at end of file diff --git a/packages/common/__test__/services/console-logger.service.spec.ts b/packages/common/__test__/services/console-logger.service.spec.ts index c82d686f..98a58721 100644 --- a/packages/common/__test__/services/console-logger.service.spec.ts +++ b/packages/common/__test__/services/console-logger.service.spec.ts @@ -1,4 +1,4 @@ -import { ConsoleLoggerService } from '../../src/services'; +import { LogLevel, ConsoleLoggerService } from '../../src'; describe('ConsoleLoggerService', () => { let logger: ConsoleLoggerService; @@ -7,16 +7,95 @@ describe('ConsoleLoggerService', () => { logger = new ConsoleLoggerService(); }); - describe('error', () => { - it('should log error', () => { - const context = 'test'; - const error = new Error('error'); + describe('setLogLevel', () => { + it('should set the log level', () => { + logger.setLogLevel(LogLevel.WARN); + expect((logger as any).logLevel).toEqual(LogLevel.WARN); + }); + }); + + describe('debug', () => { + it('should log debug when log level is DEBUG', () => { + const consoleSpy = jest.spyOn(console, 'debug').mockImplementation(); + logger.setLogLevel(LogLevel.DEBUG); + logger.debug('debug message'); + expect(consoleSpy).toHaveBeenCalledWith('debug message'); + }); + + it('should not log debug when log level is higher than DEBUG', () => { + const consoleSpy = jest.spyOn(console, 'debug').mockImplementation(); + logger.setLogLevel(LogLevel.INFO); + logger.debug('debug message'); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + }); + describe('info', () => { + it('should log info when log level is lower than INFO', () => { const consoleSpy = jest.spyOn(console, 'log').mockImplementation(); + logger.setLogLevel(LogLevel.DEBUG); + logger.info('info message'); + expect(consoleSpy).toHaveBeenCalledWith('info message'); + }); - logger.error(context, error); + it('should log info when log level is INFO', () => { + const consoleSpy = jest.spyOn(console, 'log').mockImplementation(); + logger.setLogLevel(LogLevel.INFO); + logger.info('info message'); + expect(consoleSpy).toHaveBeenCalledWith('info message'); + }); + + it('should not log info when log level is higher than INFO', () => { + const consoleSpy = jest.spyOn(console, 'log').mockImplementation(); + logger.setLogLevel(LogLevel.WARN); + logger.info('info message'); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + }); + + describe('warn', () => { + it('should log warn when log level is lower than WARN', () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + logger.setLogLevel(LogLevel.INFO); + logger.warn('warn message'); + expect(consoleSpy).toHaveBeenCalledWith('warn message'); + }); + + it('should log warn when log level is WARN', () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + logger.setLogLevel(LogLevel.WARN); + logger.warn('warn message'); + expect(consoleSpy).toHaveBeenCalledWith('warn message'); + }); + + it('should not log warn when log level is higher than WARN', () => { + const consoleSpy = jest.spyOn(console, 'warn').mockImplementation(); + logger.setLogLevel(LogLevel.ERROR); + logger.warn('warn message'); + expect(consoleSpy).not.toHaveBeenCalled(); + }); + }); + + describe('error', () => { + it('should log error when log level is lower than ERROR', () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + logger.setLogLevel(LogLevel.WARN); + logger.error('error message'); + expect(consoleSpy).toHaveBeenCalledWith('error message'); + }); + + it('should log error when log level is ERROR', () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + logger.setLogLevel(LogLevel.ERROR); + logger.error('error message'); + expect(consoleSpy).toHaveBeenCalledWith('error message'); + }); - expect(consoleSpy).toHaveBeenCalledWith(`[${context}]`, error); + it('should not log error when log level is higher than ERROR', () => { + const consoleSpy = jest.spyOn(console, 'error').mockImplementation(); + logger.setLogLevel(LogLevel.ERROR); + logger.error('error message'); + expect(consoleSpy).toHaveBeenCalledWith('error message'); }); }); }); diff --git a/packages/common/src/enums/index.ts b/packages/common/src/enums/index.ts index e8f43a4f..6ea9a367 100644 --- a/packages/common/src/enums/index.ts +++ b/packages/common/src/enums/index.ts @@ -1,3 +1,4 @@ export * from './client.enum'; export * from './event.enum'; +export * from './log.enum'; export * from './message.enum'; diff --git a/packages/common/src/enums/log.enum.ts b/packages/common/src/enums/log.enum.ts new file mode 100644 index 00000000..ee27ada0 --- /dev/null +++ b/packages/common/src/enums/log.enum.ts @@ -0,0 +1,6 @@ +export enum LogLevel { + DEBUG = 0, + INFO = 1, + WARN = 2, + ERROR = 3, +} diff --git a/packages/common/src/interfaces/event-respository.interface.ts b/packages/common/src/interfaces/event-repository.interface.ts similarity index 100% rename from packages/common/src/interfaces/event-respository.interface.ts rename to packages/common/src/interfaces/event-repository.interface.ts diff --git a/packages/common/src/interfaces/index.ts b/packages/common/src/interfaces/index.ts index 51c92e13..bef5ba94 100644 --- a/packages/common/src/interfaces/index.ts +++ b/packages/common/src/interfaces/index.ts @@ -1,6 +1,6 @@ export * from './client.interface'; export * from './common.interface'; -export * from './event-respository.interface'; +export * from './event-repository.interface'; export * from './event.interface'; export * from './filter.interface'; export * from './logger.interface'; diff --git a/packages/common/src/interfaces/logger.interface.ts b/packages/common/src/interfaces/logger.interface.ts index e1c31b13..c51d5739 100644 --- a/packages/common/src/interfaces/logger.interface.ts +++ b/packages/common/src/interfaces/logger.interface.ts @@ -1,3 +1,9 @@ +import { LogLevel } from '../enums'; + export interface Logger { - error(context: string, error: Error): void; + setLogLevel(level: LogLevel): void; + debug(message: string, ...args: any[]): void; + info(message: string, ...args: any[]): void; + warn(message: string, ...args: any[]): void; + error(message: string, ...args: any[]): void; } diff --git a/packages/common/src/services/console-logger.service.ts b/packages/common/src/services/console-logger.service.ts index c39c1b6f..7487501a 100644 --- a/packages/common/src/services/console-logger.service.ts +++ b/packages/common/src/services/console-logger.service.ts @@ -1,7 +1,34 @@ +import { LogLevel } from '../enums'; import { Logger } from '../interfaces'; export class ConsoleLoggerService implements Logger { - error(context: string, error: Error): void { - console.log(`[${context}]`, error); + private logLevel: LogLevel = LogLevel.INFO; + + setLogLevel(level: LogLevel): void { + this.logLevel = level; + } + + debug(...args: any[]): void { + if (this.logLevel <= LogLevel.DEBUG) { + console.debug(...args); + } + } + + info(...args: any[]): void { + if (this.logLevel <= LogLevel.INFO) { + console.log(...args); + } + } + + warn(...args: any[]): void { + if (this.logLevel <= LogLevel.WARN) { + console.warn(...args); + } + } + + error(...args: any[]): void { + if (this.logLevel <= LogLevel.ERROR) { + console.error(...args); + } } } diff --git a/packages/core/__test__/services/event.service.spec.ts b/packages/core/__test__/services/event.service.spec.ts index 2bcbbbe7..599ade7c 100644 --- a/packages/core/__test__/services/event.service.spec.ts +++ b/packages/core/__test__/services/event.service.spec.ts @@ -1,6 +1,7 @@ import { ClientContext, ClientReadyState, + ConsoleLoggerService, Event, EventKind, EventRepository, @@ -25,17 +26,18 @@ describe('eventService', () => { find: jest.fn(), findOne: jest.fn(), }; - subscriptionService = new SubscriptionService(new Map()); + subscriptionService = new SubscriptionService( + new Map(), + new ConsoleLoggerService(), + ); subscriptionService.broadcast = jest.fn(); pluginManagerService = new PluginManagerService(); eventService = new EventService( eventRepository, subscriptionService, pluginManagerService, + new ConsoleLoggerService(), { - logger: { - error: jest.fn(), - }, filterResultCacheTtl: 0, }, ); @@ -79,6 +81,7 @@ describe('eventService', () => { eventRepository, subscriptionService, pluginManagerService, + new ConsoleLoggerService(), ); const filters = [{}, {}] as Filter[]; const events = [{ id: 'a' }, { id: 'b' }] as Event[]; @@ -207,13 +210,16 @@ describe('eventService', () => { jest.spyOn(eventRepository, 'upsert').mockImplementation(() => { throw new Error('test'); }); + const spyLoggerError = jest + .spyOn(eventService['logger'], 'error') + .mockImplementation(); expect(await eventService.handleEvent(ctx, event)).toEqual({ success: false, message: 'error: test', }); expect(subscriptionService.broadcast).not.toHaveBeenCalled(); - expect(eventService['logger'].error).toHaveBeenCalled(); + expect(spyLoggerError).toHaveBeenCalled(); }); it('should catch unknown error', async () => { @@ -222,13 +228,16 @@ describe('eventService', () => { jest.spyOn(eventRepository, 'findOne').mockResolvedValue(null); jest.spyOn(EventUtils, 'validate').mockReturnValue(undefined); jest.spyOn(eventRepository, 'upsert').mockRejectedValue('unknown'); + const spyLoggerError = jest + .spyOn(eventService['logger'], 'error') + .mockImplementation(); expect(await eventService.handleEvent(ctx, event)).toEqual({ success: false, message: 'error: unknown', }); expect(subscriptionService.broadcast).not.toHaveBeenCalled(); - expect(eventService['logger'].error).toHaveBeenCalled(); + expect(spyLoggerError).toHaveBeenCalled(); }); }); diff --git a/packages/core/__test__/services/subscription.service.spec.ts b/packages/core/__test__/services/subscription.service.spec.ts index 26b13612..259a1ed0 100644 --- a/packages/core/__test__/services/subscription.service.spec.ts +++ b/packages/core/__test__/services/subscription.service.spec.ts @@ -2,6 +2,7 @@ import { Client, ClientContext, ClientReadyState, + ConsoleLoggerService, Event, EventUtils, Filter, @@ -17,11 +18,10 @@ describe('SubscriptionService', () => { beforeEach(() => { clientsMap = new Map(); - subscriptionService = new SubscriptionService(clientsMap, { - logger: { - error: jest.fn(), - }, - }); + subscriptionService = new SubscriptionService( + clientsMap, + new ConsoleLoggerService(), + ); client = { readyState: ClientReadyState.OPEN, send: jest.fn(), @@ -138,12 +138,15 @@ describe('SubscriptionService', () => { jest.spyOn(EventUtils, 'isMatchingFilter').mockImplementation(() => { throw new Error('error'); }); + const spyLoggerError = jest + .spyOn(subscriptionService['logger'], 'error') + .mockImplementation(); subscriptionService.subscribe(ctx, subscriptionId, filters); await subscriptionService.broadcast(event); expect(client.send).not.toHaveBeenCalled(); - expect(subscriptionService['logger'].error).toHaveBeenCalled(); + expect(spyLoggerError).toHaveBeenCalled(); }); }); }); diff --git a/packages/core/src/interfaces/handle-result.interface.ts b/packages/core/src/interfaces/handle-result.interface.ts index f89fa86e..e9d4c7ac 100644 --- a/packages/core/src/interfaces/handle-result.interface.ts +++ b/packages/core/src/interfaces/handle-result.interface.ts @@ -1,4 +1,4 @@ -import { Event, Logger, MessageType } from '@nostr-relay/common'; +import { Event, LogLevel, Logger, MessageType } from '@nostr-relay/common'; /** * Options for NostrRelay @@ -13,6 +13,10 @@ export type NostrRelayOptions = { * Logger to use. `Default: ConsoleLoggerService` */ logger?: Logger; + /** + * The minimum log level to log. `Default: LogLevel.INFO` + */ + logLevel?: LogLevel; createdAtUpperLimit?: number; createdAtLowerLimit?: number; /** diff --git a/packages/core/src/nostr-relay.ts b/packages/core/src/nostr-relay.ts index b211d296..159b0cf4 100644 --- a/packages/core/src/nostr-relay.ts +++ b/packages/core/src/nostr-relay.ts @@ -1,6 +1,7 @@ import { Client, ClientContext, + ConsoleLoggerService, Event, EventHandleResult, EventId, @@ -9,6 +10,7 @@ import { Filter, FilterUtils, IncomingMessage, + LogLevel, MessageType, NostrRelayPlugin, SubscriptionId, @@ -57,16 +59,20 @@ export class NostrRelay { ) { this.options = options; + const logger = options.logger ?? new ConsoleLoggerService(); + logger.setLogLevel(options.logLevel ?? LogLevel.INFO); + this.pluginManagerService = new PluginManagerService(); - this.subscriptionService = new SubscriptionService(this.clientContexts, { - logger: options.logger, - }); + this.subscriptionService = new SubscriptionService( + this.clientContexts, + logger, + ); this.eventService = new EventService( eventRepository, this.subscriptionService, this.pluginManagerService, + logger, { - logger: options.logger, createdAtUpperLimit: options.createdAtUpperLimit, createdAtLowerLimit: options.createdAtLowerLimit, minPowDifficulty: options.minPowDifficulty, @@ -319,7 +325,7 @@ export class NostrRelay { } /** - * Check whether a client is authorized. If NIP-42 is unabled, this method + * Check whether a client is authorized. If NIP-42 is unable, this method * always returns true. * * @param client Client instance, usually a WebSocket diff --git a/packages/core/src/services/event.service.ts b/packages/core/src/services/event.service.ts index 4acf660a..194f8165 100644 --- a/packages/core/src/services/event.service.ts +++ b/packages/core/src/services/event.service.ts @@ -1,6 +1,5 @@ import { ClientContext, - ConsoleLoggerService, Event, EventHandleResult, EventKind, @@ -15,7 +14,6 @@ import { PluginManagerService } from './plugin-manager.service'; import { SubscriptionService } from './subscription.service'; type EventServiceOptions = { - logger?: Logger; createdAtUpperLimit?: number; createdAtLowerLimit?: number; minPowDifficulty?: number; @@ -38,12 +36,13 @@ export class EventService { eventRepository: EventRepository, subscriptionService: SubscriptionService, pluginManagerService: PluginManagerService, + logger: Logger, options: EventServiceOptions = {}, ) { this.eventRepository = eventRepository; this.subscriptionService = subscriptionService; this.pluginManagerService = pluginManagerService; - this.logger = options.logger ?? new ConsoleLoggerService(); + this.logger = logger; this.createdAtUpperLimit = options.createdAtUpperLimit; this.createdAtLowerLimit = options.createdAtLowerLimit; this.minPowDifficulty = options.minPowDifficulty; @@ -99,13 +98,20 @@ export class EventService { } return await this.handleRegularEvent(ctx, event); } catch (error) { - this.logger.error(`${EventService.name}.handleEvent`, error); if (error instanceof Error) { + this.logger.error( + `[${EventService.name}.handleEvent] ${error.message}`, + error, + ); return { success: false, message: 'error: ' + error.message, }; } + this.logger.error( + `[${EventService.name}.handleEvent] unknown error`, + error, + ); return { success: false, message: 'error: unknown', diff --git a/packages/core/src/services/subscription.service.ts b/packages/core/src/services/subscription.service.ts index 80c76617..6a1b4049 100644 --- a/packages/core/src/services/subscription.service.ts +++ b/packages/core/src/services/subscription.service.ts @@ -1,7 +1,6 @@ import { Client, ClientContext, - ConsoleLoggerService, Event, EventUtils, Filter, @@ -9,21 +8,11 @@ import { } from '@nostr-relay/common'; import { createOutgoingEventMessage } from '../utils'; -type SubscriptionServiceOptions = { - logger?: Logger; -}; - export class SubscriptionService { - private readonly logger: Logger; - private readonly clientsMap: Map; - constructor( - clientsMap: Map, - options: SubscriptionServiceOptions = {}, - ) { - this.clientsMap = clientsMap; - this.logger = options.logger ?? new ConsoleLoggerService(); - } + private readonly clientsMap: Map, + private readonly logger: Logger, + ) {} subscribe( ctx: ClientContext, @@ -56,7 +45,10 @@ export class SubscriptionService { }); } } catch (error) { - this.logger.error(`${SubscriptionService.name}.eventListener`, error); + this.logger.error( + `[${SubscriptionService.name}.eventListener] ${error.message}`, + error, + ); } } } diff --git a/tsconfig.spec.json b/tsconfig.spec.json new file mode 100644 index 00000000..95af0850 --- /dev/null +++ b/tsconfig.spec.json @@ -0,0 +1,5 @@ +{ + "extends": "./tsconfig.json", + "include": ["integration/**/*", "**/*.spec.ts"], + "exclude": ["node_modules", "dist"] +} \ No newline at end of file