fix: enhance error handling in run command by restoring original stdout/stderr before outputting error messages

[ogabrielluiz]

This was causing tests to fail because they just check that something outputs to stdout.

E.g:

FAILED tests/unit/cli/test_run_starter_projects.py::TestRunStarterProjects::test_run_starter_project_format_options[News Aggregator.json] - AssertionError: No output for News Aggregator.json with format json
assert 0 > 0
 +  where 0 = len('')
 +    where '' = <Result SystemExit(1)>.output
FAILED tests/unit/cli/test_run_starter_projects.py::TestRunStarterProjects::test_run_starter_project_format_options[Pok\xe9dex Agent.json] - AssertionError: No output for Pokédex Agent.json with format json
assert 0 > 0
 +  where 0 = len('')
 +    where '' = <Result SystemExit(1)>.output
FAILED tests/unit/cli/test_run_starter_projects.py::TestRunStarterProjects::test_run_starter_project_format_options[Price Deal Finder.json] - AssertionError: No output for Price Deal Finder.json with format json
assert 0 > 0
 +  where 0 = len('')
 +    where '' = <Result SystemExit(1)>.output

Summary by CodeRabbit

• Bug Fixes
  • Restores standard output and error streams before reporting execution errors, ensuring complete and correctly ordered messages.
  • Prevents lingering redirected output after failures, keeping the terminal state consistent.
  • Improves readability of error logs in both interactive use and scripted runs.
  • Resolves cases where messages could appear missing or suppressed during failures.
  • No changes to commands or options; behavior is unchanged during successful runs.

[coderabbitai]

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.

Walkthrough

Exception handling in the CLI graph execution path now restores stdout and stderr before calling output_error and re-raising. Restoration occurs inside the except block; the finally block is unchanged.

Changes

Cohort / File(s)	Summary of Changes
CLI run error handling src/lfx/src/lfx/cli/run.py	In the exception path, restore stdout/stderr immediately before output_error(...) and re-raise; no changes to exported interfaces; finally block retained as-is.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant User
  participant CLI as CLI run.py
  participant Exec as Graph Executor
  participant Log as output_error

  User->>CLI: invoke run
  CLI->>Exec: execute_graph()
  alt success
    Exec-->>CLI: result
    CLI-->>User: normal completion
  else error
    Exec--x CLI: raise Exception
    rect rgba(255,235,205,0.5)
      note right of CLI: New behavior in error path
      CLI->>CLI: restore stdout/stderr
      CLI->>Log: output_error(err)
      CLI--x User: propagate exception
    end
  end
  opt finally
    note over CLI: finalize/cleanup (unchanged)
  end

Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Pre-merge checks and finishing touches

❌ Failed checks (1 error, 2 warnings)

Check name	Status	Explanation	Resolution
Test Coverage For New Implementations	❌ Error	No new or updated test files accompany the change that restores stdout and stderr before emitting CLI errors, despite this being a bug fix where a regression test should assert the corrected behavior; the PR touches only src/lfx/src/lfx/cli/run.py, and there are no corresponding additions or modifications under the project's test naming conventions.	Please add a regression test that fails without the stdout/stderr restoration but passes with it, saved following the existing test naming conventions (for example, a test_test_run.py covering the CLI run command), and update the PR; once the test is in place, this check can be reassessed.
Test Quality And Coverage	⚠️ Warning	No new or updated tests accompany the change that restores stdout and stderr before emitting errors, and existing coverage does not clearly exercise this corrected behavior, leaving the primary functionality untested. Without tests verifying that error messages now appear on the expected streams, we lack evidence that the fix works and that regressions will be caught, so the test quality and coverage requirements are not met.	Please add focused tests that trigger the error path in lfx.cli.run, confirm stdout and stderr are restored, and assert that error output is directed to the correct stream before resubmitting.
Test File Naming And Structure	⚠️ Warning	Repository inspection shows that there are no tests covering the lfx CLI run command path: no Python files named test_*.py target this module, and no integration or frontend tests exercise positive or negative error-handling scenarios for the change made in src/lfx/src/lfx/cli/run.py. Because required coverage and structure for this functionality are absent, the custom check fails.	Add appropriately named pytest modules (e.g., test_run.py) under the backend test suite that include logically organized tests for both success and error conditions of the lfx CLI run command, ensuring proper setup and teardown as needed.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	The title succinctly describes the main change by indicating that the fix enhances error handling in the run command through restoring stdout and stderr prior to emitting error messages, clearly reflecting the core modification and matching the pull request objectives and code changes.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Excessive Mock Usage Warning	✅ Passed	No test files were modified in this pull request, so there is no new or altered usage of mocks to evaluate, and the existing code changes do not introduce any mock-related concerns.

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[edwinjosechittilappilly]

LGTM

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 24.14%. Comparing base (ba272d7) to head (3b631d6).
⚠️ Report is 2 commits behind head on main.

❌ Your project status has failed because the head coverage (47.06%) is below the target coverage (55.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #10186      +/-   ##
==========================================
+ Coverage   24.12%   24.14%   +0.02%     
==========================================
  Files        1088     1088              
  Lines       40132    40093      -39     
  Branches     5561     5548      -13     
==========================================
  Hits         9680     9680              
+ Misses      30281    30242      -39     
  Partials      171      171              

Flag	Coverage Δ
backend	47.06% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.
see 5 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.