Skip to content

fix(headless): redirect stderr to log file via dup2#126

Merged
eitsupi merged 9 commits intomainfrom
feature/headless-stderr-redirect
Mar 21, 2026
Merged

fix(headless): redirect stderr to log file via dup2#126
eitsupi merged 9 commits intomainfrom
feature/headless-stderr-redirect

Conversation

@eitsupi
Copy link
Copy Markdown
Owner

@eitsupi eitsupi commented Mar 21, 2026

Summary

  • When --log-file is specified in headless mode, redirect the process's stderr fd to the log file using dup2
  • This ensures all stderr output — eprintln!(), R's WriteConsoleEx default output (e.g., graphics device callbacks), and any other fd 2 writes — goes to the log file instead of leaking to the terminal
  • Previously, R packages that emit diagnostics via message()/warning() during device callbacks (outside IPC evaluate context) would write to the terminal since these bypass the IPC capture mechanism

Changes

  • Add redirect_stderr parameter to init_logger() (enabled only in headless mode)
  • redirect_stderr_to_file() for Unix (libc::dup2) and Windows (open_osfhandle + dup2)
  • Update test readiness detection to use polling when --log-file is used (stderr pipe disconnected by dup2)
  • Update test_headless_log_file to verify status messages appear in the log file
  • CHANGELOG entry

Test plan

  • All 23 headless tests pass
  • cargo clippy clean
  • cargo fmt clean

🤖 Generated with Claude Code

eitsupi and others added 2 commits March 21, 2026 11:38
When --log-file is specified in headless mode, redirect the process's
stderr file descriptor to the log file using dup2. This ensures that
all stderr output — not just log::* macros but also eprintln!() calls,
R's WriteConsoleEx default output (e.g., from graphics device callbacks),
and any other code writing to fd 2 — goes to the log file instead of
leaking to the terminal.

Previously, R packages that emit diagnostics via message()/warning()
during device callbacks (outside IPC evaluate context) would write to
the terminal, since these outputs bypass the IPC capture mechanism.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Use DuplicateHandle to create an independent OS handle before passing
it to _open_osfhandle, preventing the double-close that would occur
when both the File object and the C runtime close the same handle.

Also close the intermediate fd after dup2 since fd 2 now has its own
reference, and fix a misleading comment about ordering constraints
in the Unix path.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves headless-mode logging by ensuring that when --log-file is provided, all stderr output is redirected into the log file (not just log::* output), preventing diagnostics from leaking to the terminal.

Changes:

  • Extend init_logger() with a redirect_stderr flag and implement platform-specific stderr redirection to the log file.
  • Adjust headless test readiness detection to poll when stderr is suppressed/redirected, and update log-file test assertions accordingly.
  • Add a CHANGELOG entry describing the new headless --log-file stderr redirection behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

File Description
crates/arf-console/src/main.rs Add redirect_stderr support to logger init and implement stderr dup2 redirection helpers.
crates/arf-console/tests/headless_tests.rs Update readiness detection logic for --log-file and assert status output is written to the log file.
CHANGELOG.md Document stderr redirection behavior for headless --log-file.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

eitsupi and others added 3 commits March 21, 2026 11:43
Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
HANDLE is *mut c_void on Windows, not usize.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
…mment

- Replace log::warn! with eprintln! in redirect_stderr_to_file since
  the logger has not been initialized yet at the call site
- Add doc comment to Windows variant clarifying that only the CRT fd
  is redirected (Win32 STD_ERROR_HANDLE is left unchanged)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

The default flags (0 = O_RDONLY) would make fd 2 read-only after dup2,
causing CRT writes to stderr to fail silently. Use O_APPEND to match
the append-mode log file.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Add O_WRONLY flag to open_osfhandle on Windows so fd 2 is not
  effectively read-only on some CRTs
- Rename test variable from `quiet` to `poll_for_readiness` to clarify
  its purpose covers both --quiet and --log-file cases

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Assert stderr pipe is empty (trim().is_empty()) instead of checking
  for absence of a specific substring
- Clarify comment: --quiet suppresses messages, --log-file disconnects
  the pipe — different mechanisms, same polling fallback

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@eitsupi eitsupi merged commit 7380c88 into main Mar 21, 2026
14 checks passed
@eitsupi eitsupi deleted the feature/headless-stderr-redirect branch March 21, 2026 12:29
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.

2 participants