fix(uv): log stdout on uv pip install error

[pyranota]

Important

Logs both stdout and stderr on uv pip install error in handle_python_reqs() in python_executor.rs for improved debugging.

• Behavior:
  • Logs both stdout and stderr on uv pip install error in handle_python_reqs() in python_executor.rs.
  • Captures stdout and stderr using read_to_string() and logs them if the process exits unsuccessfully.
• Error Handling:
  • If uv pip install fails, logs include both stderr and stdout outputs for better debugging.
• Code Changes:
  • Introduces stdout_buf and stdout_pipe alongside existing stderr counterparts.
  • Updates async block to await both stderr_future and stdout_future.

^{This description was created by for 7838049. You can customize this summary. It will automatically update as commits are pushed.}

[claude]

Claude finished @pyranota's task —— View job

---

Pull Request Review

Reviewing the changes to improve uv pip install error logging...

Todo List:

• Read and analyze the modified python_executor.rs file
• Evaluate code quality and best practices
• Check for potential bugs or issues
• Assess performance considerations
• Review security implications
• Provide inline comments for specific areas
• Create reproduction instructions for testers

Code Quality Analysis

✅ Overall Assessment: Good improvement with solid implementation

The changes enhance debugging capabilities by capturing both stdout and stderr from uv pip install operations, which is valuable for troubleshooting Python dependency installation issues.

Detailed Review

Strengths:

1. Improved Error Visibility: The change addresses a real debugging pain point by logging both stdout and stderr when uv pip install fails.

2. Clean Implementation: The code follows existing patterns and maintains consistency with the codebase style.

3. Proper Async Handling: Uses tokio::select! correctly to handle cancellation and await both stdout/stderr futures properly.

4. Resource Management: Properly takes ownership of both stdout and stderr pipes before spawning the async operations.

Areas for Improvement:

1. Error Handling at Line 1936-1940: The error handling for taking stdout/stderr could be more robust:

   // Current
   .ok_or(anyhow!("Cannot take stdout from uv_install_proccess"))?
   
   // Consider more descriptive error
   .ok_or_else(|| anyhow!("Failed to capture stdout pipe from uv pip install process for package: {}", req))?

2. Performance Consideration: The change now reads both stdout and stderr into memory simultaneously. For very verbose package installations, this could increase memory usage. However, this is acceptable given the debugging benefits.

3. Variable Naming at Line 1930: Using Default::default() for string initialization is less clear than explicit initialization:

   // Current
   let (mut stderr_buf, mut stdout_buf) = Default::default();
   
   // More explicit
   let (mut stderr_buf, mut stdout_buf) = (String::new(), String::new());

Security Implications:

✅ No security concerns identified

The changes only affect logging output and don't introduce any security vulnerabilities. The stdout/stderr content is already being captured securely within the sandboxed environment.

Performance Considerations:

• Memory Usage: Slight increase due to capturing stdout in addition to stderr
• I/O Efficiency: Good - both streams are read concurrently using async operations
• Network Impact: None

Inline Comments:

python_executor.rs:1930-1940 - Consider more explicit initialization and better error messages for pipe capture failures.

python_executor.rs:1979 - Good improvement in error message formatting, clearly separating stderr and stdout outputs.

python_executor.rs:1962-1966 - Excellent async pattern usage, properly awaiting both futures in the tokio::select! block.

Testing Coverage

The changes are focused on error reporting paths, which means they primarily affect failure scenarios. This is good defensive programming that will help with production debugging.

Reproduction Instructions for Testers

To test the enhanced error logging, create a Python script with intentionally problematic dependencies that will cause uv pip install to fail. For example, create a script that requires a non-existent package or has conflicting version constraints. When the script runs, the job logs should now display both stderr (traditional error messages) and stdout (additional uv output like progress indicators, warnings, or resolution details) in the error output, providing more comprehensive debugging information than the previous stderr-only logging. Test this by navigating to the Script Editor, creating a Python script with invalid requirements, and running it to observe the enhanced error details in the job execution logs.

[ellipsis-dev]

Important

Looks good to me! 👍

Reviewed everything up to 7838049 in 1 minute and 26 seconds. Click for details.

• Reviewed 54 lines of code in 1 files
• Skipped 0 files when reviewing.
• Skipped posting 3 draft comments. View those below.
• Modify your settings and rules to customize what types of comments Ellipsis leaves. And don't forget to react with 👍 or 👎 to teach Ellipsis.

1. backend/windmill-worker/src/python_executor.rs:1961

• Draft comment:
  Good job capturing stdout output for error logging. Consider awaiting both stdout and stderr concurrently (e.g. using tokio::join!) rather than sequentially, to avoid potential deadlock issues when one stream produces large output. This ensures both pipes are drained concurrently.
• Reason this comment was not posted:
  Comment looked like it was already resolved.

2. backend/windmill-worker/src/python_executor.rs:1932

• Draft comment:
  Typo: The variable name uv_install_proccess seems to be misspelled. Consider renaming it to uv_install_process if that is the intended spelling.
• Reason this comment was not posted:
  Comment was on unchanged code.

3. backend/windmill-worker/src/python_executor.rs:1965

• Draft comment:
  There's a typographical error: 'uv_install_proccess' should be spelled 'uv_install_process'.
• Reason this comment was not posted:
  Comment was on unchanged code.

Workflow ID: wflow_HE1vvfaB9ZLV0OEN

^{You can customize by changing your verbosity settings, reacting with 👍 or 👎, replying to comments, or adding code review rules.}