chore: upgrade pino to v10.1.0 and pino-http to v11.0.0 #41
Conversation
📝 WalkthroughWalkthroughConverts BuffLog to ES module imports, upgrades pino/pino-http, removes redact-object and KEYS_TO_REDACT, passes context directly to pino, updates middleware, adds Jest tests for API and redaction, and adjusts package.json and tsconfig for testing and build. Changes
Sequence Diagram(s)sequenceDiagram
participant App as Application Code
participant BuffLog as BuffLog API
participant Pino as pino Logger
participant PinoHttp as pino-http Middleware
rect rgb(235, 245, 255)
App->>BuffLog: call debug/info/... (message, context)
BuffLog->>Pino: logger.<level>(message, context)
Pino-->>BuffLog: emits structured JSON log
end
rect rgb(245, 255, 235)
App->>PinoHttp: attach middleware()
PinoHttp->>Pino: log request/response (includes req headers/body/query/cookies)
Pino-->>PinoHttp: emits structured JSON log (redact rules applied)
end
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro Disabled knowledge base sources:
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
bufflog.ts (1)
10-26: Harden LOG_LEVEL parsing and fallback.Current apply-based lowercasing is odd and doesn’t guard invalid values. Simplify and ensure fallback to "notice" if not a supported custom level.
- level: process.env.LOG_LEVEL ? String.prototype.toLowerCase.apply(process.env.LOG_LEVEL) : "notice", + level: (() => { + const raw = process.env.LOG_LEVEL?.toLowerCase(); + const allowed = new Set(['debug','info','notice','warn','error','fatal']); + return raw && allowed.has(raw) ? raw : 'notice'; + })(),
🧹 Nitpick comments (5)
tsconfig.json (2)
2-5: Tighten TS file globs; drop negation in include.Negated patterns in "include" don’t add value here. Rely on "exclude" for tests and include all TS sources.
- "include": [ - "*.ts", - "!*.test.ts" - ], + "include": ["**/*.ts"],
9-11: Revisit emit targets: ES5 + CJS is conservative for Node 18+/pino v10.If consumers run on Node 18+, consider a more modern target for smaller/faster output and better source maps; keep CJS if that’s your publish strategy.
- "target": "es5", - "module": "commonjs", + "target": "ES2019", + "module": "commonjs",Please confirm if consumers require ES5. If not, we can bump target safely.
package.json (1)
37-38: Use prepublishOnly (or prepare) instead of prepublish.prepublish runs on install in older npm; prefer prepublishOnly to limit to publish lifecycle.
- "prepublish": "./node_modules/typescript/bin/tsc" + "prepublishOnly": "./node_modules/typescript/bin/tsc"Optionally add:
+ "engines": { "node": ">=18" }to reflect pino v10 requirements.
bufflog.ts (2)
16-26: Type the logger to include custom level “notice”.Ensure TS knows about pinoLogger.notice; avoids implicit any or missing-property errors.
type CustomLevels = 'debug'|'info'|'notice'|'warn'|'error'|'fatal'; const pinoLogger: pino.Logger<CustomLevels> = pino({ /* config as-is */ });If generics are awkward with current types, minimally:
const pinoLogger = pino({ /* config */ }) as pino.Logger & { notice: pino.LogFn };Please confirm no TS errors around logger.notice in strict mode.
42-65: Avoid emitting"context": undefined.Only attach context when provided to reduce noise and downstream schema drift.
-export function debug(message: string, context?: object) { - pinoLogger.debug({context}, message); -} +export function debug(message: string, context?: Record<string, unknown>) { + context ? pinoLogger.debug({ context }, message) : pinoLogger.debug(message); +} @@ -export function info(message: string, context?: object) { - pinoLogger.info({context}, message); -} +export function info(message: string, context?: Record<string, unknown>) { + context ? pinoLogger.info({ context }, message) : pinoLogger.info(message); +} @@ -export function notice(message: string, context?: object) { - pinoLogger.notice({context}, message); -} +export function notice(message: string, context?: Record<string, unknown>) { + context ? pinoLogger.notice({ context }, message) : pinoLogger.notice(message); +} @@ -export function warning(message: string, context?: object) { - pinoLogger.warn({context}, message); -} +export function warning(message: string, context?: Record<string, unknown>) { + context ? pinoLogger.warn({ context }, message) : pinoLogger.warn(message); +} @@ -export function error(message: string, context?: object) { - pinoLogger.error({context}, message); -} +export function error(message: string, context?: Record<string, unknown>) { + context ? pinoLogger.error({ context }, message) : pinoLogger.error(message); +} @@ -export function critical(message: string, context?: object) { - pinoLogger.fatal({context}, message); -} +export function critical(message: string, context?: Record<string, unknown>) { + context ? pinoLogger.fatal({ context }, message) : pinoLogger.fatal(message); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
bufflog.test.ts(1 hunks)bufflog.ts(2 hunks)jest.config.js(1 hunks)package.json(2 hunks)tsconfig.json(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
bufflog.test.ts (1)
bufflog.ts (1)
middleware(67-82)
🔇 Additional comments (6)
tsconfig.json (1)
71-74: Exclude looks good.Tests and node_modules are excluded; aligns with ts-jest handling test transpile separately.
bufflog.test.ts (1)
15-52: Delegation coverage looks good.Level methods correctly delegate with a context envelope; expectations match the call signature.
Consider adding a case where context is omitted to ensure no
"context": undefinedis emitted.bufflog.ts (2)
27-36: Redaction config looks consistent.Merges req/res and context paths; censor token consistent with tests.
Confirm the arrays in ./constants match the policy tested in bufflog.test.ts (headers, cookies, password).
67-82: ✓ pino-http@11 customLogLevel signature confirmed.The customLogLevel signature (req, res, err) is correct, and the function correctly returns valid pino log level strings ('info' and 'error'). The logic properly maps 4xx responses to 'info', 5xx/errors to 'error', and all other cases to 'info'.
jest.config.js (1)
1-9: No compatibility gap—[email protected] supports [email protected] ✓[email protected] declares peer support for Jest 30 via peerDependencies: "jest": "^29.0.0 || ^30.0.0", so the jest.config.js requires no changes.
package.json (1)
21-31: Remove @types/pino; ts-jest 29.x already supports Jest 30.Pino provides its own type definitions, making @types/pino a stub entry that is no longer needed. However, no change to ts-jest is required: ts-jest 29.x peerDependencies explicitly allow jest ^29.0.0 || ^30.0.0, so the current
"ts-jest": "^29.4.5"is already compatible with"jest": "^30.2.0".Remove only
@types/pino.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
bufflog.test.ts (2)
88-154: Comprehensive redaction validation for documented sensitive paths.The test cases thoroughly verify redaction behavior for
req.headers,req.body.password,req.cookies, andreq.query.passwordwhile confirming non-sensitive fields remain intact. Testing both positive and negative cases (lines 138-153) is excellent practice.Consider adding assertions for log message presence in output (e.g., verify
logs[0].msg === 'Test with sensitive data') to ensure complete log structure validation.
53-56: Consider adding integration test for middleware redaction behavior.While the middleware function signature is validated, there's no test verifying that the returned middleware properly redacts sensitive data in HTTP request/response contexts. Since
middleware()is a core export that integrates with pino-http, consider adding a test that:
- Creates mock req/res objects with sensitive data
- Invokes the middleware
- Captures and validates that pino-http's output is properly redacted
This would provide confidence that the pinoHttp configuration correctly inherits BuffLog's redact settings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
bufflog.test.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
bufflog.test.ts (1)
bufflog.ts (1)
middleware(67-82)
🔇 Additional comments (2)
bufflog.test.ts (2)
3-57: Excellent API coverage for BuffLog public surface.The test suite systematically validates each logging method (debug, info, notice, warning, error, critical) with proper spy assertions and context wrapping. The setup with
beforeEachensures test isolation.
59-86: Previous concern resolved: now testing actual BuffLog logger output.This implementation addresses the past review feedback perfectly. By spying on
process.stdout.writeand parsing JSON output, the tests now exercise BuffLog's exported logger end-to-end rather than creating a separate pino instance. The logger level management ensures test logs are captured while maintaining proper isolation.
Upgrades pino from v5.17.0 to v10.1.0 and pino-http from v5.0.0 to v11.0.0 to bring the library up to date with the latest versions.
Changes
Dependencies
pinofrom ^5.16.0 to 10.1.0 (5 major versions)pino-httpfrom ~5.0.0 to 11.0.0 (6 major versions)redact-objectdependency (now using pino's built-in redaction exclusively)Code Changes
require()to ES6 imports for consistency_req, res, err)base: {}configuration (no longer needed)Testing
No Breaking Changes
None - The API remains 100% backward compatible. All existing consumers can upgrade
without code changes.
Backward Compatibility
debug,info,notice,warning,error,critical)Testing
Version
Bumped from 0.6.0 → 0.7.0
Summary by CodeRabbit
New Features
Tests
Chores