Skip to content

Fix: keep internal runtime logs silent unless debug log is enabled#38

Open
guysmoilov wants to merge 3 commits intomainfrom
feat/issue-37-debug-log-default
Open

Fix: keep internal runtime logs silent unless debug log is enabled#38
guysmoilov wants to merge 3 commits intomainfrom
feat/issue-37-debug-log-default

Conversation

@guysmoilov
Copy link
Member

@guysmoilov guysmoilov commented Feb 15, 2026

User description

Summary

  • make internal backend logger a no-op when no debug log file path is configured
  • keep file-backed logging behavior unchanged when --debug-log or TMUX_MOBILE_DEBUG_LOG is set
  • add backend unit tests covering silent default behavior and file logging behavior

Testing

  • npx vitest run tests/backend/file-logger.test.ts
  • npm test

Closes #37


PR Type

Bug fix, Tests


Description

  • Replace console with no-op logger when debug log disabled

  • Prevent internal runtime logs from appearing in stdout/stderr

  • Add comprehensive unit tests for logger behavior


Diagram Walkthrough

flowchart LR
  A["createLogger called"] --> B{"logFilePath provided?"}
  B -->|No| C["Return NOOP_LOGGER"]
  B -->|Yes| D["Return file-backed logger"]
  C --> E["Silent operation"]
  D --> F["Write to file"]
Loading

File Walkthrough

Relevant files
Bug fix
file-logger.ts
Implement no-op logger for silent default behavior             

src/backend/util/file-logger.ts

  • Added NOOP_LOGGER constant with no-op implementations of log and error
    methods
  • Changed createLogger to return NOOP_LOGGER instead of console when no
    log file path is provided
  • Maintains existing file-backed logging behavior when debug log path is
    configured
+6/-1     
Tests
file-logger.test.ts
Add unit tests for logger silent and file modes                   

tests/backend/file-logger.test.ts

  • Added new test file with comprehensive test suite for file logger
  • Test verifies logger is silent when no debug log file is configured
  • Test verifies logs are written to file with proper formatting when
    path is provided
  • Uses vitest spies and temporary directories for isolated testing
+36/-0   

Changes

src/backend/util/file-logger.ts

  • Introduces a NOOP-style DEFAULT_LOGGER that silences logger.log while still delegating logger.error to console.error so errors continue to be reported to stderr by default.
  • serialize now includes Error.stack when present (falls back to "name: message"), so error stack traces are preserved for file-backed logging.
  • createLogger returns DEFAULT_LOGGER when no logFilePath is provided; when a path is provided behavior is unchanged (creates directories, appends timestamped lines with [INFO] or [ERROR]).

tests/backend/file-logger.test.ts (new file)

Adds tests covering:

  1. Silent default behavior: logger.log is suppressed and logger.error is forwarded to console.error when no log file is configured.
  2. File logging behavior: messages are written to the configured log file with expected formatting.
  3. Error stack tracing: error.stack contents are written into the debug log.

No public API or exported-signature changes.

@coderabbitai
Copy link

coderabbitai bot commented Feb 15, 2026

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • ✅ Review completed - (🔄 Check again to review again)
📝 Walkthrough

Walkthrough

Modified the file logger to return a no-op logger when debug logging is disabled, silencing internal runtime logs by default. Added tests verifying silent mode operation and file-based logging functionality when a debug log path is configured.

Changes

Cohort / File(s) Summary
File Logger Implementation
src/backend/util/file-logger.ts
Introduced NOOP_LOGGER with no-op log and error methods. Modified createLogger to return NOOP_LOGGER instead of global console when logFilePath is undefined.
File Logger Tests
tests/backend/file-logger.test.ts
Added test suite covering silent mode (no console output when debug log disabled) and file logging mode (logs written to file when debug log path provided).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Merge Conflict Detection ⚠️ Warning ❌ Merge conflicts detected (1 file):

⚔️ src/backend/util/file-logger.ts (content)

These conflicts must be resolved before merging into main.
Resolve conflicts locally and push changes to this branch.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The pull request successfully implements all acceptance criteria from issue #37: silent logging by default with NOOP_LOGGER, file-backed logging when configured, and comprehensive test coverage.
Out of Scope Changes check ✅ Passed All changes directly address the requirements in issue #37. The modifications to file-logger.ts implement the silent default behavior, and the new test file validates both silent and file-logging modes.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/issue-37-debug-log-default

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@qodo-code-review
Copy link

qodo-code-review bot commented Feb 15, 2026

PR Compliance Guide 🔍

Below is a summary of compliance checks for this PR:

Security Compliance
🟢
No security concerns identified No security vulnerabilities detected by AI analysis. Human verification advised for critical code.
Ticket Compliance
🟡
🎫 #37
🟢 By default, internal logger output is silent (no internal runtime/debug logs printed to
stdout/stderr).
Providing --debug-log or TMUX_MOBILE_DEBUG_LOG writes internal logs to the configured
file.
Existing user-facing startup output (URLs/QR/password) remains unchanged.
Codebase Duplication Compliance
Codebase context is not defined

Follow the guide to enable codebase context checks.

Custom Compliance
🟢
Generic: Meaningful Naming and Self-Documenting Code

Objective: Ensure all identifiers clearly express their purpose and intent, making code
self-documenting

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Error Handling

Objective: To prevent the leakage of sensitive system information through error messages while
providing sufficient detail for internal debugging.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Secure Logging Practices

Objective: To ensure logs are useful for debugging and auditing without exposing sensitive
information like PII, PHI, or cardholder data.

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Security-First Input Validation and Data Handling

Objective: Ensure all data inputs are validated, sanitized, and handled securely to prevent
vulnerabilities

Status: Passed

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Comprehensive Audit Trails

Objective: To create a detailed and reliable record of critical system actions for security analysis
and compliance.

Status:
Logging silenced by default: The logger now becomes a no-op when logFilePath is undefined, which could inadvertently
disable audit-relevant logging if this logger is used for critical actions elsewhere in
the system.

Referred Code
export const createLogger = (
  logFilePath: string | undefined
): Pick<Console, "log" | "error"> => {
  if (!logFilePath) {
    return NOOP_LOGGER;
  }

Learn more about managing compliance generic rules or creating your own custom rules

Generic: Robust Error Handling and Edge Case Management

Objective: Ensure comprehensive error handling that provides meaningful context and graceful
degradation

Status:
Potentially swallowed errors: Returning NOOP_LOGGER causes all log/error calls to be silently dropped when logFilePath
is not set, which may hide important runtime error context depending on how this logger is
used.

Referred Code
const NOOP_LOGGER: Pick<Console, "log" | "error"> = {
  log: () => undefined,
  error: () => undefined
};

Learn more about managing compliance generic rules or creating your own custom rules

  • Update
Compliance status legend 🟢 - Fully Compliant
🟡 - Partial Compliant
🔴 - Not Compliant
⚪ - Requires Further Human Verification
🏷️ - Compliance label

@qodo-code-review
Copy link

qodo-code-review bot commented Feb 15, 2026

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Prevent logging failures crashing

Add a try...catch block around the fs.appendFileSync call to handle potential
file system errors and prevent the application from crashing if logging fails.

src/backend/util/file-logger.ts [33-38]

 const write = (level: "INFO" | "ERROR", values: unknown[]): void => {
-  const line = `${new Date().toISOString()} [${level}] ${values
-    .map((value) => serialize(value))
-    .join(" ")}`
-  fs.appendFileSync(resolvedPath, `${line}\n`, "utf8");
+  try {
+    const line = `${new Date().toISOString()} [${level}] ${values
+      .map((value) => serialize(value))
+      .join(" ")}`
+    fs.appendFileSync(resolvedPath, `${line}\n`, "utf8");
+  } catch (err) {
+    console.error("Failed to write log", err);
+  }
 };

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 8

__

Why: This suggestion correctly identifies that an unhandled file system error in the logging function could crash the application and proposes adding a try...catch block, which significantly improves the logger's robustness.

Medium
General
Include error stack trace

Modify the serialize function to include the stack trace when logging Error
objects, which will provide more context for debugging.

src/backend/util/file-logger.ts [9-12]

 const serialize = (value: unknown): string => {
   if (value instanceof Error) {
-    return `${value.name}: ${value.message}`;
+    return value.stack ?? `${value.name}: ${value.message}`;
   }
   // ...
 };

[To ensure code accuracy, apply this suggestion manually]

Suggestion importance[1-10]: 7

__

Why: The suggestion improves debuggability by including the stack trace when logging Error objects, providing more context for troubleshooting.

Medium
Make the shared logger object immutable

Make the NOOP_LOGGER object immutable by wrapping it with Object.freeze() to
prevent accidental runtime modifications.

src/backend/util/file-logger.ts [4-7]

-const NOOP_LOGGER: Pick<Console, "log" | "error"> = {
+const NOOP_LOGGER: Pick<Console, "log" | "error"> = Object.freeze({
   log: () => undefined,
   error: () => undefined
-};
+});
  • Apply / Chat
Suggestion importance[1-10]: 4

__

Why: The suggestion correctly recommends making the NOOP_LOGGER object immutable using Object.freeze(), which is a good defensive programming practice for shared constants, enhancing code robustness.

Low
  • Update

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Silence runtime debug logs unless debug logging is enabled

1 participant