-
-
Notifications
You must be signed in to change notification settings - Fork 412
feat: add built-in logger middleware #1601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Implements a first-class logger middleware for Elysia with: - HTTP method, path, status code, and response duration logging - Clean integration with Elysia's lifecycle hooks - Minimal performance overhead - Zero dependencies - Full TypeScript support Closes elysiajs#1599
WalkthroughThis PR introduces a built-in HTTP request logger middleware for the Elysia framework. The implementation adds a customizable logger with support for custom output functions, pattern-based path skipping, colored terminal output, and request duration tracking. It includes comprehensive tests, example usage, and exports configuration. Changes
Sequence DiagramsequenceDiagram
participant Client
participant Elysia Server
participant Logger Middleware
participant Route Handler
participant Output Function
Client->>Elysia Server: HTTP Request
Elysia Server->>Logger Middleware: onBeforeHandle
Logger Middleware->>Logger Middleware: Record start time (WeakMap)
Logger Middleware->>Route Handler: Pass through
Route Handler->>Elysia Server: Generate Response
Elysia Server->>Logger Middleware: onAfterHandle
Logger Middleware->>Logger Middleware: Compute duration<br/>Build LogEntry<br/>Check skip patterns
alt Path not skipped
Logger Middleware->>Output Function: Log entry (colored or plain)
Output Function->>Output Function: Format & display
end
Logger Middleware->>Client: Return Response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
it's to mention that more than 50% of the code here was generated with AI however i crossed checked and made sure it doesn't cause harm. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (4)
src/logger.ts (2)
89-93: Duration formatting edge case: sub-microsecond values.When
duration < 1(i.e., < 1ms), you multiply by 1000 to get microseconds. However, ifdurationis extremely small (e.g., 0.0001ms), the result would be0μs. Consider a floor of 1μs or showing decimal microseconds for accuracy.const formatDuration = (duration: number): string => { - if (duration < 1) return `${(duration * 1000).toFixed(0)}μs` + if (duration < 1) { + const us = duration * 1000 + return us < 1 ? '<1μs' : `${us.toFixed(0)}μs` + } if (duration < 1000) return `${duration.toFixed(2)}ms` return `${(duration / 1000).toFixed(2)}s` }
122-143: Pattern matching logic works but has a minor redundancy.The final
if (pattern.includes('*'))block on line 137 is only reachable for patterns with*in the middle (e.g.,/users/*/posts). The regex approach handles this correctly.However, the current logic creates a new
RegExpon every call for wildcard patterns. For frequently called paths, consider caching compiled regexes if skip patterns are static.For patterns like
/users/*/posts, a compiled regex could be cached during logger initialization:+const compilePattern = (pattern: string): ((path: string) => boolean) => { + if (!pattern.includes('*')) return (path) => pattern === path + if (pattern.endsWith('/**')) { + const prefix = pattern.slice(0, -3) + return (path) => path === prefix || path.startsWith(prefix + '/') + } + if (pattern.endsWith('/*')) { + const prefix = pattern.slice(0, -2) + return (path) => { + if (!path.startsWith(prefix + '/')) return false + const rest = path.slice(prefix.length + 1) + return !rest.includes('/') + } + } + const regex = new RegExp('^' + pattern.replace(/\*/g, '[^/]+') + '$') + return (path) => regex.test(path) +}test/logger/logger.test.ts (2)
236-257: Duration threshold may be flaky in CI.The test expects
duration >= 45for a 50ms delay. While 5ms buffer is reasonable, slow CI environments might occasionally fail. Consider increasing the delay or widening the threshold.it('measures duration correctly', async () => { const logs: LogEntry[] = [] const app = new Elysia() .use( logger({ output: (entry) => logs.push(entry) }) ) .get('/slow', async () => { - await delay(50) + await delay(100) return 'Slow response' }) await app.handle(req('/slow')) await delay(10) expect(logs.length).toBe(1) - // Duration should be at least 50ms due to delay - expect(logs[0].duration).toBeGreaterThanOrEqual(45) + // Duration should be at least 100ms due to delay + expect(logs[0].duration).toBeGreaterThanOrEqual(90) })
339-355: Default logger tests only verify no-throw behavior.These tests ensure the default logger and
colored: falsedon't throw, but they don't verify actual console output. Consider capturingconsole.logto validate output format.it('uses default logger when no output specified', async () => { const originalLog = console.log const logs: string[] = [] console.log = (...args) => logs.push(args.join(' ')) try { const app = new Elysia().use(logger()).get('/', () => 'Hello') await app.handle(req('/')) await delay(10) expect(logs.length).toBe(1) expect(logs[0]).toContain('GET') expect(logs[0]).toContain('/') } finally { console.log = originalLog } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
example/logger.ts(1 hunks)package.json(1 hunks)src/logger.ts(1 hunks)test/logger/logger.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
test/logger/logger.test.ts (2)
src/logger.ts (2)
LogEntry(3-9)logger(197-232)test/utils.ts (2)
req(1-2)delay(69-70)
example/logger.ts (1)
src/logger.ts (1)
logger(197-232)
🔇 Additional comments (8)
src/logger.ts (3)
1-11: Well-structured public API.The
LogEntryinterface andLoggerFunctiontype are clean and provide the essential fields for HTTP request logging. The interface is appropriately minimal while covering all stated requirements (method, path, status, duration, timestamp).
13-51: Good documentation and flexibility in LoggerOptions.The JSDoc examples are helpful, and supporting both
string[]and a predicate function forskipprovides good flexibility.
197-231: Core logger implementation is solid.Good use of
WeakMapfor timing storage (prevents memory leaks), and usingonAfterResponsewith{ as: 'global' }ensures logging works across plugins as intended. The early return whenstartTime === undefinedis a safe guard.One minor observation: the
skipcheck happens after fetching the start time but before computing duration. This is fine for correctness, but you could move the skip check toonRequestto avoid storing timing for skipped paths entirely.package.json (1)
165-170: Export entry follows existing conventions.The new
./loggerexport is consistent with other module exports in the package, providing types, ESM, and CJS entry points.test/logger/logger.test.ts (3)
1-6: Clean test setup with appropriate imports.Good use of test utilities (
req,post,delay) from the shared utils file.
8-29: Comprehensive basic test case.The test validates all
LogEntryfields and ensures proper logging for a simple GET request.
7-377: Well-structured and comprehensive test suite.The tests cover the key functionality: basic logging, multiple HTTP methods, skip patterns (array, function, wildcards), plugins, route parameters, error handling, and 404 responses. Good job ensuring global scope works across plugins.
example/logger.ts (1)
1-10: Good example demonstrating logger usage.The example clearly shows how to integrate the logger middleware with various route types (GET, POST, route parameters).
| console.log('🦊 Server running at http://localhost:3333') | ||
| console.log('Try these endpoints:') | ||
| console.log(' GET http://localhost:3000/') | ||
| console.log(' GET http://localhost:3000/users/123') | ||
| console.log(' POST http://localhost:3000/users') | ||
| console.log(' GEeT http://localhost:3000/health') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Port mismatch and typo in console output.
The server listens on port 3333 (line 10) but the console output references port 3000. Also, there's a typo "GEeT" on line 17.
console.log('🦊 Server running at http://localhost:3333')
console.log('Try these endpoints:')
-console.log(' GET http://localhost:3000/')
-console.log(' GET http://localhost:3000/users/123')
-console.log(' POST http://localhost:3000/users')
-console.log(' GEeT http://localhost:3000/health')
+console.log(' GET http://localhost:3333/')
+console.log(' GET http://localhost:3333/users/123')
+console.log(' POST http://localhost:3333/users')
+console.log(' GET http://localhost:3333/health')📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log('🦊 Server running at http://localhost:3333') | |
| console.log('Try these endpoints:') | |
| console.log(' GET http://localhost:3000/') | |
| console.log(' GET http://localhost:3000/users/123') | |
| console.log(' POST http://localhost:3000/users') | |
| console.log(' GEeT http://localhost:3000/health') | |
| console.log('🦊 Server running at http://localhost:3333') | |
| console.log('Try these endpoints:') | |
| console.log(' GET http://localhost:3333/') | |
| console.log(' GET http://localhost:3333/users/123') | |
| console.log(' POST http://localhost:3333/users') | |
| console.log(' GET http://localhost:3333/health') |
🤖 Prompt for AI Agents
In example/logger.ts around lines 12 to 17, the console messages incorrectly
reference port 3000 and contain a typo ("GEeT"); update all endpoint URLs to use
port 3333 to match the server and correct "GEeT" to "GET" so the printed
endpoints are accurate and consistent.
| const duration = performance.now() - startTime | ||
| const status = (set.status as number) ?? 200 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find files in src directory to understand the structure
find src -type f -name "*.ts" | head -20Repository: elysiajs/elysia
Length of output: 174
🌐 Web query:
Elysia set.status type definition how does it handle string status names
💡 Result:
Short answer: set.status (Context.set.status) is a legacy, mutable setter that accepts either a numeric HTTP status code or the common status-text string (e.g. "I'm a teapot"). It will set that status for the response, but it does not provide the same TypeScript type-narrowing/inference as the newer status()/error helpers — so using set.status won't let Elysia infer response types for different codes. [1][2]
Sources:
- Elysia handler docs (set.status examples and note about string names + lack of inference). [1]
- Elysia blog/docs recommending error/status helpers for proper type inference (shows set.status usage and limitations). [2]
Status code extraction doesn't handle string status names correctly.
The cast (set.status as number) ?? 200 doesn't actually convert string values to numbers. Elysia's set.status accepts both numeric codes and string status names (e.g., 'Created', 'OK', 'I'm a teapot'), so if a handler sets set.status = 'Created', the logger will record the string 'Created' instead of the numeric code 201. The cast to as number only affects TypeScript's type checking, not runtime behavior.
🤖 Prompt for AI Agents
In src/logger.ts around lines 219-220, the code currently uses (set.status as
number) which doesn't convert string status names to numeric codes; update the
logic to determine a numeric status: if typeof set.status === 'number' use it,
if it's a string perform a reverse lookup against Node's http.STATUS_CODES
(case-insensitive) to find the matching numeric code (parseInt the key), and if
no match fall back to 200; ensure you handle numeric strings by trying
Number(set.status) first and use the resolved numeric status for logging.
Summary
Implements a first-class logger middleware for Elysia as proposed in #1599.
Features
Usage
Closes #1599
Summary by CodeRabbit
New Features
Documentation
Tests
✏️ Tip: You can customize this high-level summary in your review settings.