From 0fe541f71ac5c037308d4aa40b57d6ed3c329f3a Mon Sep 17 00:00:00 2001 From: Martin Varmuza Date: Wed, 22 May 2024 09:21:11 +0200 Subject: [PATCH] chore(suite-desktop): create facade over ILogger to make it compatible with Log --- packages/node-utils/src/http.ts | 40 ++++--------------- packages/node-utils/src/tests/http.test.ts | 7 ++-- .../src/libs/http-receiver.ts | 6 ++- .../suite-desktop-core/src/modules/bridge.ts | 17 ++++---- .../src/utils/IloggerToLog.ts | 32 +++++++++++++++ packages/transport-bridge/src/bin.ts | 8 +++- packages/transport-bridge/src/http.ts | 7 ++-- 7 files changed, 65 insertions(+), 52 deletions(-) create mode 100644 packages/suite-desktop-core/src/utils/IloggerToLog.ts diff --git a/packages/node-utils/src/http.ts b/packages/node-utils/src/http.ts index 52ca4482e7f..259638728a4 100644 --- a/packages/node-utils/src/http.ts +++ b/packages/node-utils/src/http.ts @@ -3,7 +3,7 @@ import * as net from 'net'; import * as url from 'url'; import type { RequiredKey } from '@trezor/type-utils'; -import { LogMessage, TypedEmitter } from '@trezor/utils'; +import { Log, TypedEmitter } from '@trezor/utils'; import { arrayPartition } from '@trezor/utils'; import { getFreePort } from './getFreePort'; @@ -11,22 +11,6 @@ import { getFreePort } from './getFreePort'; type Request = RequiredKey; type EventMap = { [event: string]: any }; -type LogFn = (message: string | string[]) => void; -type Logger = { - info: LogFn; - warn: LogFn; - error: LogFn; - getLog: () => LogMessage[]; -}; - -type OriginalLogFn = (topic: string, message: string | string[]) => void; -type OriginalLogger = { - info: OriginalLogFn; - warn: OriginalLogFn; - error: OriginalLogFn; - getLog?: () => LogMessage[]; -}; - type RequestWithParams = Request & { params: Record; }; @@ -42,7 +26,7 @@ export type Handler = ( request: R, response: Response, next: Next, - { logger }: { logger: Logger }, + { logger }: { logger: Log }, ) => void; type Route = { @@ -66,28 +50,18 @@ type BaseEvents = { */ export class HttpServer extends TypedEmitter { public server: http.Server; - public logger: Logger; + public logger: Log; private routes: Route[] = []; private readonly emitter: TypedEmitter = this; private port?: number; private sockets: Record = {}; - constructor({ logger, port }: { logger: OriginalLogger; port?: number }) { + constructor({ logger, port }: { logger: Log; port?: number }) { super(); this.port = port; - // this class accepts subset of suite-desktop-core "ILogger" interface. - // - in order to omit need for passing the first argument "topic" in each call, we wrap the logger and prepend "http: ${this.port}" to each call - // - here it implements also only a subset of ILogger functionality - // - todo: unify loggers across the codebase - this.logger = { - info: (message: string | string[]) => logger.info(`${this.logName}`, message), - warn: (message: string | string[]) => logger.warn(`${this.logName}`, message), - error: (message: string | string[]) => logger.error(`${this.logName}`, message), - getLog: () => (logger.getLog ? logger.getLog() : []), - }; - // this.logger = logger; + this.logger = logger; this.server = http.createServer(this.onRequest); } @@ -337,7 +311,7 @@ const checkOrigin = ({ request: Parameters[0]; allowedOrigin: string[]; pathname: string; - logger: Logger; + logger: Log; }) => { const { origin } = request.headers; const origins = allowedOrigin ?? []; @@ -373,7 +347,7 @@ const checkReferer = ({ request: Parameters[0]; allowedReferer: string[]; pathname: string; - logger: Logger; + logger: Log; }) => { const { referer } = request.headers; const referers = allowedReferer ?? []; diff --git a/packages/node-utils/src/tests/http.test.ts b/packages/node-utils/src/tests/http.test.ts index 05b1099f8a1..0f23385409f 100644 --- a/packages/node-utils/src/tests/http.test.ts +++ b/packages/node-utils/src/tests/http.test.ts @@ -14,6 +14,7 @@ describe('HttpServer', () => { let server: HttpServer; beforeEach(() => { server = new HttpServer({ + // @ts-expect-error logger: muteLogger, }); }); @@ -45,6 +46,7 @@ describe('HttpServer', () => { }); test('a port can be passed to the constructor', async () => { + // @ts-expect-error server = new HttpServer({ logger: muteLogger, port: 65526 }); await server.start(); expect(server.getServerAddress()).toMatchObject({ @@ -119,10 +121,7 @@ describe('HttpServer', () => { await new Promise(resolve => setTimeout(resolve, 500)); controller.abort(); await new Promise(resolve => setTimeout(resolve, 500)); - expect(muteLogger.info).toHaveBeenLastCalledWith( - expect.any(String), - 'Request GET /foo aborted', - ); + expect(muteLogger.info).toHaveBeenLastCalledWith('Request GET /foo aborted'); }); test('allowReferer middleware lets request with allowed referer through', async () => { diff --git a/packages/suite-desktop-core/src/libs/http-receiver.ts b/packages/suite-desktop-core/src/libs/http-receiver.ts index 1f1c5126ba3..722ed18401b 100644 --- a/packages/suite-desktop-core/src/libs/http-receiver.ts +++ b/packages/suite-desktop-core/src/libs/http-receiver.ts @@ -4,6 +4,7 @@ import { xssFilters } from '@trezor/utils'; import { HttpServer, allowReferers } from '@trezor/node-utils'; import { HTTP_ORIGINS_DEFAULT } from './constants'; +import { convertILoggerToLog } from '../utils/IloggerToLog'; type TemplateOptions = { title?: string; @@ -38,7 +39,10 @@ const applyTemplate = (content: string, options?: TemplateOptions) => { }; export const createHttpReceiver = () => { - const httpReceiver = new HttpServer({ logger: global.logger, port: 21335 }); + const httpReceiver = new HttpServer({ + logger: convertILoggerToLog(global.logger, { serviceName: 'http-receiver' }), + port: 21335, + }); httpReceiver.use([ (request, response, next) => { diff --git a/packages/suite-desktop-core/src/modules/bridge.ts b/packages/suite-desktop-core/src/modules/bridge.ts index dc366987515..4a34c9ac442 100644 --- a/packages/suite-desktop-core/src/modules/bridge.ts +++ b/packages/suite-desktop-core/src/modules/bridge.ts @@ -6,6 +6,8 @@ import { TrezordNode } from '@trezor/transport-bridge'; import { app, ipcMain } from '../typed-electron'; import { BridgeProcess } from '../libs/processes/BridgeProcess'; import { b2t } from '../libs/utils'; +import { Logger } from '../libs/logger'; +import { convertILoggerToLog } from '../utils/IloggerToLog'; import type { Module, Dependencies } from './index'; @@ -53,20 +55,17 @@ const getBridgeInstance = (store: Dependencies['store']) => { return new BridgeProcess(); } + const bridgeLogger = new Logger('info', { + writeToDisk: false, + writeToMemory: true, + }); + return new TrezordNode({ port: 21325, api: bridgeDev || bridgeTest ? 'udp' : 'usb', assetPrefix: '../build/node-bridge', // passing down ILogger where Log is expected. - // @ts-expect-error - logger: { - ...global.logger, - log: (...args) => logger.info('trezord-node', args.join(' ')), - info: (...args) => logger.info('trezord-node', args.join(' ')), - warn: (...args) => logger.warn('trezord-node', args.join(' ')), - debug: (...args) => logger.debug('trezord-node', args.join(' ')), - error: (...args) => logger.error('trezord-node', args.join(' ')), - }, + logger: convertILoggerToLog(bridgeLogger, { serviceName: 'trezord-node' }), }); }; diff --git a/packages/suite-desktop-core/src/utils/IloggerToLog.ts b/packages/suite-desktop-core/src/utils/IloggerToLog.ts new file mode 100644 index 00000000000..64968b3b477 --- /dev/null +++ b/packages/suite-desktop-core/src/utils/IloggerToLog.ts @@ -0,0 +1,32 @@ +import { Log, LogMessage as UtilsLogMessage } from '@trezor/utils'; + +/** take an instance of ILogger and return mimicked instance of Log while keeping more or less the same behavior */ +export const convertILoggerToLog = ( + iLogger: ILogger, + { serviceName }: { serviceName: string }, +): Log => { + return { + log: (msg: string) => iLogger.info(serviceName, msg), + info: (msg: string) => iLogger.info(serviceName, msg), + debug: (msg: string) => iLogger.debug(serviceName, msg), + warn: (msg: string) => iLogger.warn(serviceName, msg), + error: (msg: string) => iLogger.error(serviceName, msg), + prefix: '', + messages: [], + enabled: true, + css: '', + MAX_ENTRIES: 1000, + setColors: (_colors: any) => {}, + setWriter: (_logWriter: any) => {}, + addMessage: (_msg: UtilsLogMessage) => {}, + logWriter: undefined, + getLog: (): UtilsLogMessage[] => { + return iLogger.getLog().map(log => ({ + message: [log.text], + prefix: '', + level: log.level, + timestamp: log.date.getTime(), + })); + }, + }; +}; diff --git a/packages/transport-bridge/src/bin.ts b/packages/transport-bridge/src/bin.ts index 0a2be9293f5..47a38ab6e1c 100644 --- a/packages/transport-bridge/src/bin.ts +++ b/packages/transport-bridge/src/bin.ts @@ -1,5 +1,11 @@ +import { Log } from '@trezor/utils'; + import { TrezordNode } from './http'; -const trezordNode = new TrezordNode({ port: 21325, api: 'usb' }); +const trezordNode = new TrezordNode({ + port: 21325, + api: 'usb', + logger: new Log('@trezor/transport-bridge', true), +}); trezordNode.start(); diff --git a/packages/transport-bridge/src/http.ts b/packages/transport-bridge/src/http.ts index 5325601d0a9..173f3290a07 100644 --- a/packages/transport-bridge/src/http.ts +++ b/packages/transport-bridge/src/http.ts @@ -47,11 +47,10 @@ export class TrezordNode { port: number; api: 'usb' | 'udp'; assetPrefix?: string; - logger?: Log; + logger: Log; }) { this.port = port || defaults.port; - this.logger = logger || new Log('@trezor/transport-bridge', true); - + this.logger = logger; this.descriptors = []; this.listenSubscriptions = []; @@ -271,7 +270,7 @@ export class TrezordNode { intro: `To download full logs go to http://127.0.0.1:${this.port}/logs`, version: this.version, devices: enumerateResult.success ? enumerateResult.payload.descriptors : [], - logs: app.logger.getLog().slice(-20), + logs: this.logger.getLog().slice(-20), }; res.end(str(props));