Skip to content

Implement TTY detection for Hono's logger middleware#4823

Open
Goldyvaiiii wants to merge 2 commits intohonojs:mainfrom
Goldyvaiiii:fix/logger-tty-detection
Open

Implement TTY detection for Hono's logger middleware#4823
Goldyvaiiii wants to merge 2 commits intohonojs:mainfrom
Goldyvaiiii:fix/logger-tty-detection

Conversation

@Goldyvaiiii
Copy link

Currently, the logger middleware always applies ANSI color codes regardless of whether
the output destination is a terminal. This causes garbled color escape sequences in
logs when output is piped to files, CI systems, or other non-TTY destinations.

This PR adds cross-runtime TTY detection to getColorEnabled() in src/utils/color.ts:

  • Node.js / Bun: checks process.stdout.isTTY
  • Deno: checks Deno.stdout.isTerminal() (with legacy isatty() fallback)
  • Serverless / Workers: defaults to true (unchanged behavior)

The NO_COLOR env variable continues to take highest precedence.

@Goldyvaiiii
Copy link
Author

Adds TTY detection to getColorEnabled() to avoid emitting ANSI colors in non-TTY environments (e.g., CI, piped logs). Supports Node/Bun (process.stdout.isTTY) and Deno (stdout.isTerminal() with fallback). Preserves existing serverless behavior. Updates tests and adds TTY coverage.

@yusukebe
Copy link
Member

Related to #3017 #3809

@yusukebe
Copy link
Member

Hey @ryuapp @NamesMT

Is this a good time to think of this matter? If we start implementing TTY detection, I think using #3017 is better.

@Goldyvaiiii
Copy link
Author

Hey @yusukebe — thanks for the suggestion!

I've updated this PR to align with the centralized TTY detection approach from #3017.

  • Added src/utils/flags.ts for environment-based detection
  • Refactored getColorEnabled() to use these flags
  • Ensured correct precedence for NO_COLOR and FORCE_COLOR
  • Updated tests to evaluate the flags dynamically

All tests are passing locally. Please let me know if this direction looks good or if you'd like any further adjustments.

@ryuapp
Copy link
Contributor

ryuapp commented Mar 25, 2026

Hi.

Is this a good time to think of this matter? If we start implementing TTY detection, I think using #3017 is better.

I agree with the approach #3017 for now. In most cases, it's better to prevent problems from occurring by default.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants