Skip to content

fix: remove stderr-based error detection to fix false positives#4

Merged
eitsupi merged 1 commit intomainfrom
fix/install-packages-false-error-indicator
Jan 28, 2026
Merged

fix: remove stderr-based error detection to fix false positives#4
eitsupi merged 1 commit intomainfrom
fix/install-packages-false-error-indicator

Conversation

@eitsupi
Copy link
Copy Markdown
Owner

@eitsupi eitsupi commented Jan 28, 2026

No description provided.

Remove COMMAND_HAD_STDERR from error detection logic because many R
functions (e.g., install.packages) write informational messages to
stderr that are not errors.

Error detection now relies solely on R's error handling mechanism:
- CONDITION_ERROR_OCCURRED via globalCallingHandlers
- R-side error state via options(error = ...) handler

This fixes the issue where install.packages showed a failure indicator
in the prompt even when the package was installed successfully.

Co-Authored-By: Claude Opus 4.5 <[email protected]>
Copilot AI review requested due to automatic review settings January 28, 2026 16:33
@eitsupi eitsupi merged commit c5f6ab5 into main Jan 28, 2026
5 checks passed
@eitsupi eitsupi deleted the fix/install-packages-false-error-indicator branch January 28, 2026 16:33
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 refines error detection in the embedded R runtime by removing stderr-based heuristics that caused false positives, and relying instead on R’s own error-handling state.

Changes:

  • Removed COMMAND_HAD_STDERR tracking and all related logic from reset_command_error_state, command_had_error, and r_write_console_ex, so stderr output no longer influences error detection.
  • Updated the command_had_error documentation to describe the new behavior based solely on R’s condition system and options(error = ...)-driven error state, explicitly noting why stderr is not used.
  • Adjusted tests in sys.rs to validate the updated error-tracking behavior (using mark_error_condition and R-side error state) and removed tests that depended on stderr-driven error flags.

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

eitsupi added a commit that referenced this pull request Feb 11, 2026
- Fix fd leak when PENDING_TERMIOS_RESTORE mutex is poisoned after
  successful tcgetattr (finding #2)
- Restore comment explaining intentional if guard in write_empty_password
  for release builds where debug_assert is stripped (finding #3)
- Document that rpassword's internal fd leaks on longjmp as a known
  minor limitation (finding #1)
- Note UTF-8 restriction from rpassword vs previous raw bytes (finding #4)

Co-Authored-By: Claude Opus 4.6 <[email protected]>
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