Skip to content

fix: log debug output when commands fail - #77

Open
rupayon123 wants to merge 8 commits into
albertalef:masterfrom
rupayon123:debug-log-stderr-on-failure
Open

rupayon123 wants to merge 8 commits into
albertalef:masterfrom
rupayon123:debug-log-stderr-on-failure

Conversation

@rupayon123

Copy link
Copy Markdown

Fixes #73

Debug mode now logs the command details before re-raising RubyShell::CommandError, so failed commands still show what ran. I also included stderr in the debug output, as requested in the issue thread.

Checks:

  • ruby -c lib/rubyshell/debugger.rb
  • ruby -c lib/rubyshell/error.rb
  • ruby -c spec/debugger_spec.rb
  • manual debug failure check with ruby -Ilib

I could not run the full RSpec file locally because the rspec executable is not installed here.

@albertalef

Copy link
Copy Markdown
Owner

Hi!! Sorry for the delay. Im reviewing now

Comment thread lib/rubyshell/debugger.rb
rescue RubyShell::CommandError => e
time_two = Process.clock_gettime(Process::CLOCK_MONOTONIC)
log_command(command, time_two - time_one, e.status, e.stdout, e.stderr)
raise

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

In some situations on the past, i had some troubles with errors losing data when rescued.

This actual code will not give us problems, but. Lets add a test checking that when we have a command being rescued outside, with debug option true, the returned CommandError has the correct attributes. Like:

begin
  ls('error', _debug: true)
rescue => e
  puts e.command # => Correct command
  puts e.stdout # => Correct stdout
  # etc
end

@albertalef

Copy link
Copy Markdown
Owner

Nice! Now, check the cicd, there is some failures

Comment thread spec/debugger_spec.rb Outdated
end

it "logs failed command stdout before reraising" do
run_failed_debug_command

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

We can separate the run_failed_debug_command on before blocks and distinct contexts, instead create a method to execute the code. To follow the idiomatic idea of rspec

@albertalef

Copy link
Copy Markdown
Owner

@rupayon123 there are still some failures, can you check?

@rupayon123

Copy link
Copy Markdown
Author

Fixed the two RuboCop offenses from the last run in 0f35634: added the missing final newline in debugger.rb and removed the redundant begin from the spec's before block.

Both edited files pass ruby -c, git diff --check passes, and a focused debugger check confirms it preserves the original exception and its attributes, logs the failure, and returns successful results. I couldn't run the full suite locally with this Mac's Ruby 2.6; the command-path smoke check hit a keyword-argument compatibility error before reaching the debugger.

The new Ruby CI run requires maintainer approval before it can execute. Could you approve it so we can verify the full suite and RuboCop on Ruby 3.2.2?

@rupayon123

Copy link
Copy Markdown
Author

Follow-up for 585ab3a924410e4ffa38e9c4795ba8758d277d30: the symbol-option fix is pushed and I rechecked it locally. Symbol public flags render, internal _debug is omitted from the shell arguments, and an actual failing shell command preserves stdout, stderr, exit status 7 and the debug log.

This morning's attempt to post the validation note failed through the connector; this is the first posted update for that commit. Local Ruby syntax and focused smoke checks pass. Full RSpec remains unverified locally because this environment lacks the project's current Bundler/debug tooling. The new CI runs 34486079934 and 34486079865 await maintainer approval.

I also found a separate Ruby 2.6 operator-routing failure and submitted it independently in #78, keeping that patch out of this debug-error PR. Prepared with AI assistance.

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.

Debug mode when the command fails

2 participants