From f3577a16b09abb4bb241a999ed7b582d6eb19adc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?TATSUNO=20=E2=80=9CTaz=E2=80=9D=20Yasuhiro?= Date: Sat, 2 Nov 2024 16:16:47 +0900 Subject: [PATCH] perf(middleware/logger): optimize color status (#3603) * refactor(logger): optimize colors status * refactor(logger): add failing test * refactor(logger): remove unreachable cases. Response accepts 200-599 * refactor(logger): added fallback * perf(middleware/logger): cleanup tests * perf(middleware/logger): test unsupported status code forcibly --- src/middleware/logger/index.test.ts | 43 +++++++++++++++++++++++++++++ src/middleware/logger/index.ts | 27 ++++++++++-------- 2 files changed, 58 insertions(+), 12 deletions(-) diff --git a/src/middleware/logger/index.test.ts b/src/middleware/logger/index.test.ts index efc81254..7c7e4689 100644 --- a/src/middleware/logger/index.test.ts +++ b/src/middleware/logger/index.test.ts @@ -28,6 +28,17 @@ describe('Logger by Middleware', () => { return c.text(longRandomString) }) app.get('/empty', (c) => c.text('')) + app.get('/redirect', (c) => { + return c.redirect('/empty', 301) + }) + app.get('/server-error', (c) => { + const res = new Response('', { status: 511 }) + if (c.req.query('status')) { + // test status code not yet supported by runtime `Response` object + Object.defineProperty(res, 'status', { value: parseInt(c.req.query('status') as string) }) + } + return res + }) }) it('Log status 200 with empty body', async () => { @@ -62,6 +73,14 @@ describe('Logger by Middleware', () => { expect(log).toMatch(/1s/) }) + it('Log status 301 with empty body', async () => { + const res = await app.request('http://localhost/redirect') + expect(res).not.toBeNull() + expect(res.status).toBe(301) + expect(log.startsWith('--> GET /redirect \x1b[36m301\x1b[0m')).toBe(true) + expect(log).toMatch(/m?s$/) + }) + it('Log status 404', async () => { const msg = 'Default 404 Not Found' app.all('*', (c) => { @@ -73,6 +92,30 @@ describe('Logger by Middleware', () => { expect(log.startsWith('--> GET /notfound \x1b[33m404\x1b[0m')).toBe(true) expect(log).toMatch(/m?s$/) }) + + it('Log status 511 with empty body', async () => { + const res = await app.request('http://localhost/server-error') + expect(res).not.toBeNull() + expect(res.status).toBe(511) + expect(log.startsWith('--> GET /server-error \x1b[31m511\x1b[0m')).toBe(true) + expect(log).toMatch(/m?s$/) + }) + + it('Log status 100', async () => { + const res = await app.request('http://localhost/server-error?status=100') + expect(res).not.toBeNull() + expect(res.status).toBe(100) + expect(log.startsWith('--> GET /server-error 100')).toBe(true) + expect(log).toMatch(/m?s$/) + }) + + it('Log status 700', async () => { + const res = await app.request('http://localhost/server-error?status=700') + expect(res).not.toBeNull() + expect(res.status).toBe(700) + expect(log.startsWith('--> GET /server-error 700')).toBe(true) + expect(log).toMatch(/m?s$/) + }) }) describe('Logger by Middleware in NO_COLOR', () => { diff --git a/src/middleware/logger/index.ts b/src/middleware/logger/index.ts index d39f934a..abcfbec7 100644 --- a/src/middleware/logger/index.ts +++ b/src/middleware/logger/index.ts @@ -28,19 +28,22 @@ const time = (start: number) => { const colorStatus = (status: number) => { const colorEnabled = getColorEnabled() - const out: { [key: string]: string } = { - 7: colorEnabled ? `\x1b[35m${status}\x1b[0m` : `${status}`, - 5: colorEnabled ? `\x1b[31m${status}\x1b[0m` : `${status}`, - 4: colorEnabled ? `\x1b[33m${status}\x1b[0m` : `${status}`, - 3: colorEnabled ? `\x1b[36m${status}\x1b[0m` : `${status}`, - 2: colorEnabled ? `\x1b[32m${status}\x1b[0m` : `${status}`, - 1: colorEnabled ? `\x1b[32m${status}\x1b[0m` : `${status}`, - 0: colorEnabled ? `\x1b[33m${status}\x1b[0m` : `${status}`, + if (colorEnabled) { + switch ((status / 100) | 0) { + case 5: // red = error + return `\x1b[31m${status}\x1b[0m` + case 4: // yellow = warning + return `\x1b[33m${status}\x1b[0m` + case 3: // cyan = redirect + return `\x1b[36m${status}\x1b[0m` + case 2: // green = success + return `\x1b[32m${status}\x1b[0m` + } } - - const calculateStatus = (status / 100) | 0 - - return out[calculateStatus] + // Fallback to unsupported status code. + // E.g.) Bun and Deno supports new Response with 101, but Node.js does not. + // And those may evolve to accept more status. + return `${status}` } type PrintFunc = (str: string, ...rest: string[]) => void