diff --git a/lib/react_on_rails/dev/process_manager.rb b/lib/react_on_rails/dev/process_manager.rb index 8e70ad85ed..7ecae3996c 100644 --- a/lib/react_on_rails/dev/process_manager.rb +++ b/lib/react_on_rails/dev/process_manager.rb @@ -1,14 +1,18 @@ # frozen_string_literal: true +require "timeout" + module ReactOnRails module Dev class ProcessManager + # Timeout for version check operations to prevent hanging + VERSION_CHECK_TIMEOUT = 5 + class << self + # Check if a process is available and usable in the current execution context + # This accounts for bundler context where system commands might be intercepted def installed?(process) - IO.popen([process, "-v"], &:close) - true - rescue Errno::ENOENT - false + installed_in_current_context?(process) end def ensure_procfile(procfile) @@ -31,20 +35,124 @@ def run_with_process_manager(procfile) # Clean up stale files before starting FileManager.cleanup_stale_files - if installed?("overmind") - system("overmind", "start", "-f", procfile) - elsif installed?("foreman") - system("foreman", "start", "-f", procfile) + # Try process managers in order of preference + return if run_process_if_available("overmind", ["start", "-f", procfile]) + return if run_process_if_available("foreman", ["start", "-f", procfile]) + + show_process_manager_installation_help + exit 1 + end + + private + + # Check if a process is actually usable in the current execution context + # This is important for commands that might be intercepted by bundler + def installed_in_current_context?(process) + # Try to execute the process with version flags to see if it works + # Use system() because that's how we'll actually call it later + version_flags_for(process).any? do |flag| + # Add timeout to prevent hanging on version checks + Timeout.timeout(VERSION_CHECK_TIMEOUT) do + system(process, flag, out: File::NULL, err: File::NULL) + end + end + rescue Errno::ENOENT, Timeout::Error + false + end + + # Get appropriate version flags for different processes + def version_flags_for(process) + case process + when "overmind" + ["--version"] + when "foreman" + ["--version", "-v"] else - warn <<~MSG - NOTICE: - For this script to run, you need either 'overmind' or 'foreman' installed on your machine. Please try this script after installing one of them. - MSG - exit 1 + ["--version", "-v", "-V"] end end - private + # Try to run a process if it's available, with intelligent fallback strategy + # Returns true if process was found and executed, false if not available + def run_process_if_available(process, args) + # First attempt: try in current context (works for bundled processes) + return system(process, *args) if installed?(process) + + # Second attempt: try in system context (works for system-installed processes) + return run_process_outside_bundle(process, args) if process_available_in_system?(process) + + # Process not available in either context + false + end + + # Run a process outside of bundler context + # This allows using system-installed processes even when they're not in the Gemfile + def run_process_outside_bundle(process, args) + if defined?(Bundler) + with_unbundled_context do + system(process, *args) + end + else + # Fallback if Bundler is not available + system(process, *args) + end + end + + # Check if a process is available system-wide (outside bundle context) + def process_available_in_system?(process) + return false unless defined?(Bundler) + + with_unbundled_context do + # Try version flags to check if process exists outside bundler context + version_flags_for(process).any? do |flag| + # Add timeout to prevent hanging on version checks + Timeout.timeout(VERSION_CHECK_TIMEOUT) do + system(process, flag, out: File::NULL, err: File::NULL) + end + end + end + rescue Errno::ENOENT, Timeout::Error + false + end + + # DRY helper method for Bundler context switching with API compatibility + # Supports both new (with_unbundled_env) and legacy (with_clean_env) Bundler APIs + def with_unbundled_context(&block) + if Bundler.respond_to?(:with_unbundled_env) + Bundler.with_unbundled_env(&block) + elsif Bundler.respond_to?(:with_clean_env) + Bundler.with_clean_env(&block) + else + # Fallback if neither method is available (very old Bundler versions) + yield + end + end + + # Improved error message with helpful guidance + def show_process_manager_installation_help + warn <<~MSG + ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + ERROR: Process Manager Not Found + ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + + To run the React on Rails development server, you need either 'overmind' or + 'foreman' installed on your system. + + RECOMMENDED INSTALLATION: + + macOS: brew install overmind + Linux: gem install foreman + + IMPORTANT: + DO NOT add foreman to your Gemfile. Install it globally on your system. + + For more information about why foreman should not be in your Gemfile, see: + https://github.com/shakacode/react_on_rails/blob/master/docs/javascript/foreman-issues.md + + After installation, try running this script again. + ━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━ + MSG + end def valid_procfile_path?(procfile) # Reject paths with shell metacharacters diff --git a/spec/react_on_rails/dev/process_manager_spec.rb b/spec/react_on_rails/dev/process_manager_spec.rb index 978ada0bce..eda46a8352 100644 --- a/spec/react_on_rails/dev/process_manager_spec.rb +++ b/spec/react_on_rails/dev/process_manager_spec.rb @@ -4,29 +4,43 @@ require "react_on_rails/dev/process_manager" RSpec.describe ReactOnRails::Dev::ProcessManager do - # Suppress stdout/stderr during tests - before(:all) do - @original_stderr = $stderr - @original_stdout = $stdout - $stderr = File.open(File::NULL, "w") - $stdout = File.open(File::NULL, "w") - end - - after(:all) do - $stderr = @original_stderr - $stdout = @original_stdout - end - describe ".installed?" do - it "returns true when process is available" do - allow(IO).to receive(:popen).with(["overmind", "-v"]).and_return("Some version info") + it "returns true when process is available in current context" do + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).and_yield + expect_any_instance_of(Kernel).to receive(:system) + .with("overmind", "--version", out: File::NULL, err: File::NULL).and_return(true) expect(described_class).to be_installed("overmind") end - it "returns false when process is not available" do - allow(IO).to receive(:popen).with(["nonexistent", "-v"]).and_raise(Errno::ENOENT) + it "returns false when process is not available in current context" do + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).and_raise(Errno::ENOENT) expect(described_class.installed?("nonexistent")).to be false end + + it "returns false when all version flags fail" do + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).exactly(3).times.and_yield + expect_any_instance_of(Kernel).to receive(:system) + .with("failing_process", "--version", out: File::NULL, err: File::NULL).and_return(false) + expect_any_instance_of(Kernel).to receive(:system) + .with("failing_process", "-v", out: File::NULL, err: File::NULL).and_return(false) + expect_any_instance_of(Kernel).to receive(:system) + .with("failing_process", "-V", out: File::NULL, err: File::NULL).and_return(false) + expect(described_class.installed?("failing_process")).to be false + end + + it "returns true when second version flag succeeds" do + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).twice.and_yield + expect_any_instance_of(Kernel).to receive(:system) + .with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(false) + expect_any_instance_of(Kernel).to receive(:system) + .with("foreman", "-v", out: File::NULL, err: File::NULL).and_return(true) + expect(described_class.installed?("foreman")).to be true + end + + it "returns false when version check times out" do + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).and_raise(Timeout::Error) + expect(described_class.installed?("hanging_process")).to be false + end end describe ".ensure_procfile" do @@ -50,33 +64,177 @@ end it "uses overmind when available" do - allow(described_class).to receive(:installed?).with("overmind").and_return(true) - expect_any_instance_of(Kernel).to receive(:system).with("overmind", "start", "-f", "Procfile.dev") + expect(described_class).to receive(:run_process_if_available) + .with("overmind", ["start", "-f", "Procfile.dev"]).and_return(true) described_class.run_with_process_manager("Procfile.dev") end - it "uses foreman when overmind not available" do - allow(described_class).to receive(:installed?).with("overmind").and_return(false) - allow(described_class).to receive(:installed?).with("foreman").and_return(true) - expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev") + it "uses foreman when overmind not available and foreman is available" do + expect(described_class).to receive(:run_process_if_available) + .with("overmind", ["start", "-f", "Procfile.dev"]).and_return(false) + expect(described_class).to receive(:run_process_if_available) + .with("foreman", ["start", "-f", "Procfile.dev"]).and_return(true) described_class.run_with_process_manager("Procfile.dev") end it "exits with error when no process manager available" do - allow(described_class).to receive(:installed?).with("overmind").and_return(false) - allow(described_class).to receive(:installed?).with("foreman").and_return(false) + expect(described_class).to receive(:run_process_if_available) + .with("overmind", ["start", "-f", "Procfile.dev"]).and_return(false) + expect(described_class).to receive(:run_process_if_available) + .with("foreman", ["start", "-f", "Procfile.dev"]).and_return(false) + expect(described_class).to receive(:show_process_manager_installation_help) expect_any_instance_of(Kernel).to receive(:exit).with(1) described_class.run_with_process_manager("Procfile.dev") end it "cleans up stale files before starting" do - allow(described_class).to receive(:installed?).with("overmind").and_return(true) + allow(described_class).to receive(:run_process_if_available).and_return(true) expect(ReactOnRails::Dev::FileManager).to receive(:cleanup_stale_files) described_class.run_with_process_manager("Procfile.dev") end end + + describe ".run_process_if_available" do + it "returns true and runs process when available in current context" do + allow(described_class).to receive(:installed?).with("foreman").and_return(true) + expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev").and_return(true) + + result = described_class.send(:run_process_if_available, "foreman", ["start", "-f", "Procfile.dev"]) + expect(result).to be true + end + + it "tries system context when not available in current context" do + allow(described_class).to receive(:installed?).with("foreman").and_return(false) + allow(described_class).to receive(:process_available_in_system?).with("foreman").and_return(true) + expect(described_class).to receive(:run_process_outside_bundle) + .with("foreman", ["start", "-f", "Procfile.dev"]).and_return(true) + + result = described_class.send(:run_process_if_available, "foreman", ["start", "-f", "Procfile.dev"]) + expect(result).to be true + end + + it "returns false when process not available anywhere" do + allow(described_class).to receive(:installed?).with("nonexistent").and_return(false) + allow(described_class).to receive(:process_available_in_system?).with("nonexistent").and_return(false) + + result = described_class.send(:run_process_if_available, "nonexistent", ["start"]) + expect(result).to be false + end + end + + describe ".run_process_outside_bundle" do + it "uses with_unbundled_context when Bundler is available" do + expect(described_class).to receive(:with_unbundled_context).and_yield + expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev") + + described_class.send(:run_process_outside_bundle, "foreman", ["start", "-f", "Procfile.dev"]) + end + + it "falls back to direct system call when Bundler is not available" do + hide_const("Bundler") + expect_any_instance_of(Kernel).to receive(:system).with("foreman", "start", "-f", "Procfile.dev") + + described_class.send(:run_process_outside_bundle, "foreman", ["start", "-f", "Procfile.dev"]) + end + end + + describe ".process_available_in_system?" do + it "checks process availability outside bundle context with version flags" do + expect(described_class).to receive(:with_unbundled_context).and_yield + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).and_yield + expect_any_instance_of(Kernel).to receive(:system) + .with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(true) + + expect(described_class.send(:process_available_in_system?, "foreman")).to be true + end + + it "returns false when Bundler is not available" do + hide_const("Bundler") + + expect(described_class.send(:process_available_in_system?, "foreman")).to be false + end + + it "tries multiple version flags before failing" do + expect(described_class).to receive(:with_unbundled_context).and_yield + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).twice.and_yield + expect_any_instance_of(Kernel).to receive(:system) + .with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(false) + expect_any_instance_of(Kernel).to receive(:system) + .with("foreman", "-v", out: File::NULL, err: File::NULL).and_return(true) + + expect(described_class.send(:process_available_in_system?, "foreman")).to be true + end + + it "returns false when version check times out in system context" do + expect(described_class).to receive(:with_unbundled_context).and_yield + expect(Timeout).to receive(:timeout).with(described_class::VERSION_CHECK_TIMEOUT).and_raise(Timeout::Error) + + expect(described_class.send(:process_available_in_system?, "hanging_process")).to be false + end + end + + describe ".version_flags_for" do + it "returns specific flags for overmind" do + expect(described_class.send(:version_flags_for, "overmind")).to eq(["--version"]) + end + + it "returns multiple flags for foreman" do + expect(described_class.send(:version_flags_for, "foreman")).to eq(["--version", "-v"]) + end + + it "returns generic flags for unknown processes" do + expect(described_class.send(:version_flags_for, "unknown")).to eq(["--version", "-v", "-V"]) + end + end + + describe ".with_unbundled_context" do + it "uses with_unbundled_env when available" do + bundler_double = class_double(Bundler) + stub_const("Bundler", bundler_double) + allow(bundler_double).to receive(:respond_to?).with(:with_unbundled_env).and_return(true) + expect(bundler_double).to receive(:with_unbundled_env).and_yield + + yielded = false + described_class.send(:with_unbundled_context) { yielded = true } + expect(yielded).to be true + end + + it "falls back to with_clean_env when with_unbundled_env not available" do + bundler_double = class_double(Bundler) + stub_const("Bundler", bundler_double) + allow(bundler_double).to receive(:respond_to?).with(:with_unbundled_env).and_return(false) + allow(bundler_double).to receive(:respond_to?).with(:with_clean_env).and_return(true) + expect(bundler_double).to receive(:with_clean_env).and_yield + + yielded = false + described_class.send(:with_unbundled_context) { yielded = true } + expect(yielded).to be true + end + + it "yields directly when neither method is available" do + bundler_double = class_double(Bundler) + stub_const("Bundler", bundler_double) + allow(bundler_double).to receive(:respond_to?).with(:with_unbundled_env).and_return(false) + allow(bundler_double).to receive(:respond_to?).with(:with_clean_env).and_return(false) + + yielded = false + described_class.send(:with_unbundled_context) { yielded = true } + expect(yielded).to be true + end + end + + describe ".show_process_manager_installation_help" do + it "displays helpful error message with installation instructions" do + expect { described_class.send(:show_process_manager_installation_help) } + .to output(/ERROR: Process Manager Not Found/).to_stderr + expect { described_class.send(:show_process_manager_installation_help) } + .to output(/DO NOT add foreman to your Gemfile/).to_stderr + expect { described_class.send(:show_process_manager_installation_help) } + .to output(/foreman-issues\.md/).to_stderr + end + end end