Research reliable Pino logger flush on shutdown

[UniquePixels]

Context

Audit ID: SEC-7

Pino worker-thread transports have no reliable flush callback. The current 100ms delay in logger.ts may be insufficient under load, potentially losing final log lines during shutdown.

Approach

• Investigate Pino internals for flush support with worker-thread transports
• Consider increasing delay to 250-500ms as a simple fix
• Evaluate custom transport with explicit flush if log loss is unacceptable
• Document findings regardless of outcome

Files

• src/core/lib/logger/logger.ts (L117-122)

[coderabbitai]

Coding Plan

Summary

• Refactor development transport setup to use pino.transport() separately, enabling access to the stream's .end() method for reliable shutdown flushing
• Implement transport.end() with callback/Promise support in shutdown(), with a timeout fallback for robustness
• Retain production behavior unchanged (no worker-thread transport, direct stdout)
• Document findings and rationale inline, fulfilling the audit documentation requirement

Design Choices

Design Choice 1: Should we implement `transport.end()` support or just increase the delay?

Options Considered:

1. Simple fix: Increase delay from 100ms to 250-500ms without architectural changes
2. Refactor to use pino.transport() separately, enabling proper transport.end() calls with callback support

Chosen Option: 2

Rationale: Refactoring to use pino.transport() separately provides a proper flush mechanism with callback support, is a minimal change, and aligns with Pino's documented approach for reliable shutdown. The delay increase can be combined as a fallback.

💡 User Tips

Regenerate the plan with different choices with @coderabbitai <feedback>.

Implementation Steps

Phase 1: Enable Reliable Transport Flush Mechanism

Refactor the development transport configuration to use pino.transport() as a separate call, enabling access to the transport stream's .end() method for proper shutdown flushing.

Task 1: Refactor Transport Initialization

Modify the transport setup to create the transport stream separately from the pino logger instantiation, retaining a reference for shutdown use.

• In src/core/lib/logger/logger.ts, replace the inline pinoOptions.transport configuration with a separate pino.transport() call when in development mode
• Store the resulting transport stream in a variable accessible to the shutdown() closure
• Pass the transport stream as the second argument to pino() instead of using pinoOptions.transport
• Ensure production mode remains unchanged (no transport, direct stdout JSON output)

Task 2: Implement Proper Transport Shutdown

Update the shutdown() method to use transport.end() with callback support instead of relying solely on a fixed delay.

• Modify shutdown() in src/core/lib/logger/logger.ts to call transport.end() when a transport stream exists
• Wrap the .end() call in a Promise that resolves on the 'close' event or callback
• Add a timeout fallback (250-500ms) to prevent hanging if the transport fails to close
• Retain baseLogger.flush() call before ending the transport
• Keep the existing Sentry flush logic unchanged

Task 3: Update Type Definitions

Ensure type definitions accommodate the optional transport stream reference.

• Review src/core/lib/logger/types.ts to confirm no interface changes are needed (the ExtendedLogger.shutdown() signature remains unchanged)
• Add appropriate internal typing for the transport stream variable within the factory closure

🤖 Prompt for AI agents

Refactor the development transport configuration in
`src/core/lib/logger/logger.ts` to use `pino.transport()` as a separate call and
update the `shutdown()` method for reliable flushing.

**Task 1: Refactor Transport Initialization**
- In `src/core/lib/logger/logger.ts`, remove the inline `pinoOptions.transport`
configuration
- When in development mode, call `pino.transport()` separately to create a
transport stream and store it in a variable accessible to the `shutdown()`
closure
- Pass the transport stream as the second argument to `pino()` instead of via
`pinoOptions.transport`
- Leave production mode unchanged — no transport, direct stdout JSON output

**Task 2: Implement Proper Transport Shutdown**
- In the `shutdown()` method of `src/core/lib/logger/logger.ts`, call
`baseLogger.flush()` first (retain existing behavior)
- When a transport stream exists, call `transport.end()` and wrap it in a
Promise that resolves on the `'close'` event or callback
- Add a timeout fallback of 250–500ms to prevent hanging if the transport fails
to close
- Keep the existing Sentry flush logic unchanged

**Task 3: Update Type Definitions**
- Review `src/core/lib/logger/types.ts` — no changes to the
`ExtendedLogger.shutdown()` signature are expected
- Add appropriate internal typing for the optional transport stream variable
within the factory closure

Phase 2: Documentation and Testing

Document the findings from this investigation and ensure the shutdown behavior is properly tested.

Task 1: Document Flush Behavior and Findings

Add documentation explaining the flush mechanism, its limitations, and the rationale for the implementation approach.

• Add a block comment in src/core/lib/logger/logger.ts near the transport setup explaining:
  • Why pino.transport() is called separately (to enable .end())
  • The difference between dev (worker-thread transport) and prod (direct stdout)
  • The fallback timeout rationale
• Update the existing comment in shutdown() to reflect the new mechanism
• Document that SIGTERM/SIGINT handling relies on the shutdown handler in shutdown.ts rather than Pino's built-in exit hooks

Task 2: Add Shutdown Flush Tests

Extend the logger test suite to verify the transport shutdown behavior.

• In src/core/lib/logger/index.test.ts, add tests that verify shutdown() completes without hanging
• Add a test case that logs messages immediately before shutdown and verifies the shutdown promise resolves within expected bounds
• Consider adding a test that mocks a slow-closing transport to verify timeout fallback behavior

🤖 Prompt for AI agents

Add inline documentation to `src/core/lib/logger/logger.ts` and extend the
logger test suite in `src/core/lib/logger/index.test.ts` to cover the new
shutdown behavior.

**Task 1: Document Flush Behavior and Findings**
- Near the transport setup in `src/core/lib/logger/logger.ts`, add a block
comment that explains:
- Why `pino.transport()` is called separately (to expose `.end()` for reliable
shutdown)
- The distinction between dev mode (worker-thread transport via pino-pretty) and
prod mode (direct stdout JSON)
  - The rationale for the timeout fallback in shutdown
- Update the existing comment inside `shutdown()` to describe the new
`transport.end()` + Promise + timeout mechanism
- Add a note that SIGTERM/SIGINT handling is delegated to `shutdown.ts` rather
than Pino's built-in exit hooks

**Task 2: Add Shutdown Flush Tests**
- In `src/core/lib/logger/index.test.ts`, add tests verifying that `shutdown()`
resolves without hanging
- Add a test that logs messages immediately before calling `shutdown()` and
asserts the returned promise resolves within expected time bounds
- Optionally add a test that mocks a slow-closing transport stream to verify the
timeout fallback resolves correctly

Research

The logger system is built around a createLogger factory in src/core/lib/logger/logger.ts that conditionally configures pino-pretty as a worker-thread transport for development only. Production writes JSON directly to stdout without any transport. The shutdown() method currently calls baseLogger.flush() followed by a 100ms delay as a best-effort drain for worker threads. The shutdown handler in src/shutdown.ts invokes logger.shutdown() as the final cleanup step before process.exit(0). The transport is configured inline via pinoOptions.transport, meaning no transport stream reference is retained for calling .end().

---

🚀 Next Steps

🤖 All AI agent prompts combined

Task: 1

Refactor the development transport configuration in
`src/core/lib/logger/logger.ts` to use `pino.transport()` as a separate call and
update the `shutdown()` method for reliable flushing.

**Task 1: Refactor Transport Initialization**
- In `src/core/lib/logger/logger.ts`, remove the inline `pinoOptions.transport`
configuration
- When in development mode, call `pino.transport()` separately to create a
transport stream and store it in a variable accessible to the `shutdown()`
closure
- Pass the transport stream as the second argument to `pino()` instead of via
`pinoOptions.transport`
- Leave production mode unchanged — no transport, direct stdout JSON output

**Task 2: Implement Proper Transport Shutdown**
- In the `shutdown()` method of `src/core/lib/logger/logger.ts`, call
`baseLogger.flush()` first (retain existing behavior)
- When a transport stream exists, call `transport.end()` and wrap it in a
Promise that resolves on the `'close'` event or callback
- Add a timeout fallback of 250–500ms to prevent hanging if the transport fails
to close
- Keep the existing Sentry flush logic unchanged

**Task 3: Update Type Definitions**
- Review `src/core/lib/logger/types.ts` — no changes to the
`ExtendedLogger.shutdown()` signature are expected
- Add appropriate internal typing for the optional transport stream variable
within the factory closure
===============================================================================

Task: 2

Add inline documentation to `src/core/lib/logger/logger.ts` and extend the
logger test suite in `src/core/lib/logger/index.test.ts` to cover the new
shutdown behavior.

**Task 1: Document Flush Behavior and Findings**
- Near the transport setup in `src/core/lib/logger/logger.ts`, add a block
comment that explains:
- Why `pino.transport()` is called separately (to expose `.end()` for reliable
shutdown)
- The distinction between dev mode (worker-thread transport via pino-pretty) and
prod mode (direct stdout JSON)
  - The rationale for the timeout fallback in shutdown
- Update the existing comment inside `shutdown()` to describe the new
`transport.end()` + Promise + timeout mechanism
- Add a note that SIGTERM/SIGINT handling is delegated to `shutdown.ts` rather
than Pino's built-in exit hooks

**Task 2: Add Shutdown Flush Tests**
- In `src/core/lib/logger/index.test.ts`, add tests verifying that `shutdown()`
resolves without hanging
- Add a test that logs messages immediately before calling `shutdown()` and
asserts the returned promise resolves within expected time bounds
- Optionally add a test that mocks a slow-closing transport stream to verify the
timeout fallback resolves correctly

💡 Iterate on the plan with: @coderabbitai <feedback>

Example Feedback
- `@coderabbitai` You can skip phase 3. Add a simple unit test case for phase 2.
- `@coderabbitai` For design choice 1 go ahead with option 3 and replan.

---

💬 Have feedback or questions? Drop into our discord!