From 6bf8fa5fa79a1f345bd2ddec271fccd669f29a2f Mon Sep 17 00:00:00 2001 From: TheoryOfNekomata Date: Sat, 15 Jul 2023 11:46:39 +0800 Subject: [PATCH] Update logger Make logger pino-compliant. --- TODO.md | 6 + packages/cli/package.json | 1 + .../cli/src/modules/adder/adder.controller.ts | 4 +- packages/cli/src/packages/cli-wrapper.ts | 122 +++++++++++------- packages/cli/src/packages/write-stream.ts | 19 +++ packages/cli/test/index.test.ts | 4 +- .../src/modules/adder/adder.controller.ts | 23 +++- packages/web-api/src/routes.ts | 25 +++- pnpm-lock.yaml | 3 + 9 files changed, 154 insertions(+), 53 deletions(-) create mode 100644 TODO.md create mode 100644 packages/cli/src/packages/write-stream.ts diff --git a/TODO.md b/TODO.md new file mode 100644 index 0000000..64faa56 --- /dev/null +++ b/TODO.md @@ -0,0 +1,6 @@ +- We could make a common code generation strategy between + CLI and Web API since we have unified the code structure. +- What this means is we just need to declare the connection + from the core SDK to the CLI or Web API functions, then we + can run some sort of code generation, so we don't have to + bother writing the Web API/CLI code manually. diff --git a/packages/cli/package.json b/packages/cli/package.json index 64637a8..67d229d 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -67,6 +67,7 @@ "dependencies": { "@clack/prompts": "^0.6.3", "@modal-sh/core": "workspace:*", + "pino": "^8.14.1", "yargs": "^17.7.2" } } diff --git a/packages/cli/src/modules/adder/adder.controller.ts b/packages/cli/src/modules/adder/adder.controller.ts index 1431117..e602ccd 100644 --- a/packages/cli/src/modules/adder/adder.controller.ts +++ b/packages/cli/src/modules/adder/adder.controller.ts @@ -34,7 +34,7 @@ export class AdderControllerImpl implements AdderController { const { a, b } = params.args; try { const response = this.adderService.addNumbers({ a: Number(a), b: Number(b) }); - params.logger.log(response); + params.logger.info(response); } catch (errorRaw) { const error = errorRaw as Error; params.logger.error(error.message); @@ -65,7 +65,7 @@ export class AdderControllerImpl implements AdderController { const { a, b } = params.args; try { const response = this.adderService.addNumbers({ a: Number(a), b: -(Number(b)) }); - params.logger.log(response); + params.logger.info(response); } catch (errorRaw) { const error = errorRaw as Error; params.logger.error(error.message); diff --git a/packages/cli/src/packages/cli-wrapper.ts b/packages/cli/src/packages/cli-wrapper.ts index cbadfd5..ffb6c96 100644 --- a/packages/cli/src/packages/cli-wrapper.ts +++ b/packages/cli/src/packages/cli-wrapper.ts @@ -1,13 +1,10 @@ -import tty from 'tty'; -import yargs, {Argv} from 'yargs'; +import yargs, { Argv } from 'yargs'; import * as clack from '@clack/prompts'; +import { DummyWriteStream } from './write-stream'; +import pino, {LogFn} from 'pino'; +import * as util from 'util'; -export interface Logger { - log: (message: unknown) => void; - error: (message: unknown) => void; - warn: (message: unknown) => void; - debug: (message: unknown) => void; -} +export interface Logger extends pino.BaseLogger {} export interface CommandHandlerArgs { self: any; @@ -33,24 +30,6 @@ type PromptValueArgs = [Record, ...never[]] | [[string, string], ...never[]] | [[string, string][], ...never[]]; -class DummyWriteStream extends tty.WriteStream { - private bufferInternal = Buffer.from(''); - - constructor() { - super(0); - } - - write(input: Uint8Array | string, _encoding?: BufferEncoding | ((err?: Error) => void), _cb?: (err?: Error) => void): boolean { - this.bufferInternal = Buffer.concat([this.bufferInternal, Buffer.from(input)]); - // noop - return true; - } - - get buffer(): Buffer { - return this.bufferInternal; - } -} - export interface TestModeResult { exitCode?: number; stdout: DummyWriteStream; @@ -69,6 +48,7 @@ interface InteractiveModeOptions { export interface CliOptions { name: string; interactiveMode?: Partial; + logger?: Logger | boolean; } export class Cli { @@ -171,27 +151,6 @@ export class Cli { }; } - private generateLogger() { - return { - log: (message: unknown) => { - const stdout = this.testMode ? this.testModeResult.stdout : process.stdout; - stdout.write(`${message?.toString()}\n`); - }, - warn: (message: unknown) => { - const stdout = this.testMode ? this.testModeResult.stdout : process.stdout; - stdout.write(`WARN: ${message?.toString()}\n`); - }, - debug: (message: unknown) => { - const stdout = this.testMode ? this.testModeResult.stdout : process.stdout; - stdout.write(`DEBUG: ${message?.toString()}\n`); - }, - error: (message: unknown) => { - const stderr = this.testMode ? this.testModeResult.stderr : process.stderr; - stderr.write(`${message?.toString()}\n`); - }, - }; - } - private buildHandler(handlerFn: Function, interactiveModeOptions: InteractiveModeOptions) { const thisCli = this; return async function handler(this: any, commandArgs: Record) { @@ -207,11 +166,78 @@ export class Cli { interactiveModeOptions, ); + const stdoutLogFn: LogFn = (...args: unknown[]) => { + const stream = thisCli.testMode ? thisCli.testModeResult.stdout : process.stdout; + const [arg0, arg1, ...etcArgs] = args; + if (typeof arg0 === 'string' || typeof arg0 === 'number') { + if (arg1) { + if (etcArgs.length > 0) { + stream.write(util.format(`${arg0}\n`, arg1, ...etcArgs)); + } else { + stream.write(util.format(`${arg0}\n`, arg1)); + } + } else { + stream.write(util.format(`${arg0}\n`)); + } + } else if (typeof arg0 === 'object' && (typeof arg1 === 'string' || typeof arg1 === 'number')) { + if (etcArgs.length > 0) { + stream.write(util.format(`${arg1}\n`, ...etcArgs)); + } else { + stream.write(util.format(`${arg1}\n`)); + } + } + }; + + const stderrLogFn: LogFn = (...args: unknown[]) => { + const stream = thisCli.testMode ? thisCli.testModeResult.stderr : process.stderr; + const [arg0, arg1, ...etcArgs] = args; + if (typeof arg0 === 'string' || typeof arg0 === 'number') { + if (arg1) { + if (etcArgs.length > 0) { + stream.write(util.format(`${arg0}\n`, arg1, ...etcArgs)); + } else { + stream.write(util.format(`${arg0}\n`, arg1)); + } + } else { + stream.write(util.format(`${arg0}\n`)); + } + } else if (typeof arg0 === 'object' && (typeof arg1 === 'string' || typeof arg1 === 'number')) { + if (etcArgs.length > 0) { + stream.write(util.format(`${arg1}\n`, ...etcArgs)); + } else { + stream.write(util.format(`${arg1}\n`)); + } + } + }; + + const loggerFn = thisCli.options.logger; + const defaultLogger = { + level: 'info', + debug: stdoutLogFn, + info: stdoutLogFn, + warn: stdoutLogFn, + error: stderrLogFn, + fatal: stderrLogFn, + trace: stdoutLogFn, + silent: () => {}, + } as Logger; + const loggerBooleanFn = typeof loggerFn === 'boolean' && loggerFn ? defaultLogger : { + level: 'silent', + debug: () => {}, + info: () => {}, + warn: () => {}, + error: () => {}, + fatal: () => {}, + trace: () => {}, + silent: () => {}, + } as Logger; + const logger = typeof loggerFn === 'undefined' ? defaultLogger : (typeof loggerFn === 'function' ? loggerFn : loggerBooleanFn); + let exited = false; const returnCode = await handlerFn({ self, interactive, - logger: thisCli.generateLogger(), + logger, send: (code: number) => { exited = true; thisCli.exit(code); diff --git a/packages/cli/src/packages/write-stream.ts b/packages/cli/src/packages/write-stream.ts new file mode 100644 index 0000000..5b139e4 --- /dev/null +++ b/packages/cli/src/packages/write-stream.ts @@ -0,0 +1,19 @@ +import {WriteStream} from 'tty'; + +export class DummyWriteStream extends WriteStream { + private bufferInternal = Buffer.from(''); + + constructor() { + super(0); + } + + write(input: Uint8Array | string, _encoding?: BufferEncoding | ((err?: Error) => void), _cb?: (err?: Error) => void): boolean { + this.bufferInternal = Buffer.concat([this.bufferInternal, Buffer.from(input)]); + // noop + return true; + } + + get buffer(): Buffer { + return this.bufferInternal; + } +} diff --git a/packages/cli/test/index.test.ts b/packages/cli/test/index.test.ts index 69e50c1..0de508f 100644 --- a/packages/cli/test/index.test.ts +++ b/packages/cli/test/index.test.ts @@ -1,7 +1,8 @@ import { describe, it, expect, vi, beforeAll } from 'vitest'; -import { Cli, createCli } from '../src/cli'; +import { createCli } from '../src/cli'; import { addCommands } from '../src/commands'; import { AdderServiceImpl, InvalidArgumentTypeError, ArgumentOutOfRangeError } from '../src/modules/adder'; +import { Cli } from '../src/packages/cli-wrapper'; vi.mock('process'); @@ -10,6 +11,7 @@ describe('blah', () => { beforeAll(() => { cli = createCli({ name: 'cli-test', + logger: false, }); addCommands(cli); }); diff --git a/packages/web-api/src/modules/adder/adder.controller.ts b/packages/web-api/src/modules/adder/adder.controller.ts index aaa4f56..e98280b 100644 --- a/packages/web-api/src/modules/adder/adder.controller.ts +++ b/packages/web-api/src/modules/adder/adder.controller.ts @@ -9,6 +9,7 @@ import { constants } from 'http2'; export interface AdderController { addNumbers: RouteHandlerMethod; + subtractNumbers: RouteHandlerMethod; } export class AdderControllerImpl implements AdderController { @@ -21,7 +22,27 @@ export class AdderControllerImpl implements AdderController { readonly addNumbers: RouteHandlerMethod = async (request, reply) => { const { a, b } = request.body as { a: number; b: number }; try { - const response = this.adderService.addNumbers({a, b}); + const response = this.adderService.addNumbers({ a, b }); + reply.send(response); + } catch (errorRaw) { + if (errorRaw instanceof InvalidArgumentTypeError) { + request.log.info(errorRaw); + reply.status(constants.HTTP_STATUS_BAD_REQUEST).send(errorRaw.message); + return; + } + if (errorRaw instanceof ArgumentOutOfRangeError) { + reply.status(constants.HTTP_STATUS_BAD_REQUEST).send(errorRaw.message); + return; + } + const error = errorRaw as Error; + reply.status(constants.HTTP_STATUS_INTERNAL_SERVER_ERROR).send(error.message); + } + } + + readonly subtractNumbers: RouteHandlerMethod = async (request, reply) => { + const { a, b } = request.body as { a: number; b: number }; + try { + const response = this.adderService.addNumbers({ a, b: -b }); reply.send(response); } catch (errorRaw) { if (errorRaw instanceof InvalidArgumentTypeError) { diff --git a/packages/web-api/src/routes.ts b/packages/web-api/src/routes.ts index 58b7a1e..44154cc 100644 --- a/packages/web-api/src/routes.ts +++ b/packages/web-api/src/routes.ts @@ -7,7 +7,30 @@ export const addRoutes = (server: FastifyInstance) => { return server .route({ method: 'POST', - url: '/', + url: '/add', + schema: { + body: { + type: 'object', + properties: { + a: { type: 'number' }, + b: { type: 'number' }, + } + } + }, handler: adderController.addNumbers, + }) + .route({ + method: 'POST', + url: '/subtract', + schema: { + body: { + type: 'object', + properties: { + a: { type: 'number' }, + b: { type: 'number' }, + } + } + }, + handler: adderController.subtractNumbers, }); }; diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 04ed2fe..56cfbaf 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -14,6 +14,9 @@ importers: '@modal-sh/core': specifier: workspace:* version: link:../core + pino: + specifier: ^8.14.1 + version: 8.14.1 yargs: specifier: ^17.7.2 version: 17.7.2