check return code before parsing version

[adhami3310]

No description provided.

[greptile-apps]

Greptile Overview

Summary

This PR refactors JavaScript runtime version detection by creating a reusable helper function `_get_version_of_executable` that consolidates duplicate code between Node.js and Bun version detection functions. The key improvement is adding proper return code validation before attempting to parse version output.

The changes extract common version-parsing logic from get_node_version() and get_bun_version() into a shared helper that handles the complete workflow: executing the version command, checking the return code, parsing the output, and handling various error conditions. This follows the DRY principle while making the code more robust.

The helper function integrates well with Reflex's existing architecture by using the established processes.new_process() utility for command execution and the console module for consistent error reporting. The function signature allows flexibility with a configurable version_arg parameter, defaulting to --version which works for most executables.

Important Files Changed

Changed Files

Filename	Score	Overview
reflex/utils/js_runtimes.py	4/5	Refactored version detection by extracting common logic into a reusable helper function with proper return code checking

Confidence score: 4/5

• This PR is safe to merge with low risk of issues
• Score reflects good defensive programming practices and proper error handling improvements
• No files require special attention beyond standard code review

Sequence Diagram

sequenceDiagram
    participant User
    participant ValidationFunction as "validate_frontend_dependencies"
    participant JSPackageExecutor as "get_js_package_executor"
    participant NodeVersionChecker as "check_node_version"
    participant VersionExecutable as "_get_version_of_executable"
    participant ProcessRunner as "processes.new_process"
    participant Console as "console"

    User->>ValidationFunction: "Call validate_frontend_dependencies(init=False)"
    
    ValidationFunction->>JSPackageExecutor: "get_js_package_executor(raise_on_none=True)"
    JSPackageExecutor-->>ValidationFunction: "Return package managers or raise FileNotFoundError"
    
    alt FileNotFoundError raised
        ValidationFunction->>Console: "console.error()"
        ValidationFunction->>User: "raise SystemExit(1)"
    end
    
    ValidationFunction->>ValidationFunction: "Check prefer_npm_over_bun()"
    
    alt npm preferred and node check needed
        ValidationFunction->>NodeVersionChecker: "check_node_version()"
        NodeVersionChecker->>NodeVersionChecker: "get_node_version()"
        NodeVersionChecker->>VersionExecutable: "_get_version_of_executable(node_path)"
        VersionExecutable->>ProcessRunner: "new_process([executable_path, '--version'])"
        ProcessRunner-->>VersionExecutable: "Return result with returncode and stdout"
        
        alt returncode != 0
            VersionExecutable->>Console: "console.error('Failed to run ... Return code: ...')"
            VersionExecutable-->>NodeVersionChecker: "Return None"
        else returncode == 0
            VersionExecutable->>VersionExecutable: "version.parse(result.stdout.strip())"
            VersionExecutable-->>NodeVersionChecker: "Return parsed version"
        end
        
        NodeVersionChecker-->>ValidationFunction: "Return version comparison result"
        
        alt Node version invalid
            ValidationFunction->>Console: "console.error('Reflex requires node version ...')"
            ValidationFunction->>User: "raise SystemExit(1)"
        end
    end
    
    ValidationFunction-->>User: "Validation complete"

Loading

_{1 file reviewed, 2 comments}

_{Edit Code Review Agent Settings | Greptile}

[codspeed-hq]

CodSpeed Performance Report

Merging #5845 will not alter performance

_{Comparing check-return-code-before-parsing-versions (91fb65f) with main (6ea6c04)¹}

Summary

✅ 8 untouched

Footnotes

1. No successful run was found on main (1fe61cf) during the generation of this report, so 6ea6c04 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩