Skip to content

Commit 6c77213

Browse files
AbanoubGhadbanclaudegithub-actions[bot]coderabbitai[bot]
authored
Fix: Universal process manager bundler context fallback (#1833)
* Fix foreman execution in bundler context with intelligent fallback This commit resolves the issue where bin/dev fails when foreman is not included in the Gemfile (following React on Rails best practices). The fix implements an intelligent fallback strategy: 1. First attempt: Try running foreman within bundler context 2. Fallback: Use Bundler.with_unbundled_env to run system-installed foreman 3. Enhanced error handling with helpful user guidance Key improvements: - Maintains compatibility with projects that include foreman in Gemfile - Supports React on Rails best practice of system-installed foreman - Provides clear error messages linking to documentation - Comprehensive test coverage for all fallback scenarios Fixes #1832 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Enhance process manager with universal bundler context fallback This commit generalizes the foreman-specific bundler context fix to work for ALL process managers (overmind, foreman, and any future additions). Key improvements: - Context-aware process detection for all processes, not just foreman - Universal fallback mechanism using Bundler.with_unbundled_env - Consistent API with run_process(process, args) for any process manager - Enhanced test coverage for generalized functionality Benefits: - Robust handling of bundler context for any process manager - Future-proof architecture for new process managers - Consistent behavior across all system commands - Eliminates bundler interceptor issues universally Builds on previous commit to create a comprehensive solution. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Optimize process manager performance and robustness This commit improves the process manager implementation with several optimizations: Performance improvements: - Eliminate redundant system calls in run_process_if_available - Streamlined control flow with early returns - More efficient process availability checking Robustness enhancements: - Support multiple version flags per process (--version, -v, -V) - Process-specific version flag mapping for better compatibility - Improved fallback strategy that avoids double execution Code quality: - Cleaner, more readable method structure - Better separation of concerns - Enhanced test coverage for version flag handling - RuboCop compliant code style The implementation is now more efficient and robust while maintaining full backward compatibility and comprehensive bundler context handling. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Add timeout protection for version checking operations This commit adds a 5-second timeout to all version checking operations to prevent the development setup from hanging indefinitely on unresponsive process managers. Safety improvements: - Timeout protection for installed_in_current_context? checks - Timeout protection for process_available_in_system? checks - Graceful handling of Timeout::Error alongside Errno::ENOENT - Prevents hanging development setup on problematic installations Implementation details: - Uses Ruby's Timeout module with 5-second limit - Only applied to version checking, not main process execution - Main process execution remains unlimited (expected for dev servers) - Comprehensive test coverage for timeout scenarios The timeout is conservative (5 seconds) to account for slow systems while still preventing indefinite hangs that would block developer workflow. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Add Bundler API compatibility with DRY helper method Implement with_unbundled_context helper that supports both modern (with_unbundled_env) and legacy (with_clean_env) Bundler APIs, with graceful fallback for very old versions. This addresses backward compatibility concerns while maintaining clean, DRY code. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Add timeout protection for version checking operations Extract hardcoded timeout value to VERSION_CHECK_TIMEOUT constant for better maintainability and testability. Co-authored-by: Abanoub Ghadban <AbanoubGhadban@users.noreply.github.com> * Apply CodeRabbit review suggestions - Extract process managers to PROCESS_MANAGERS constant for DRY code - Relax procfile path validation (remove shell metacharacter check) - Refactor tests to use around hook with ensure clause for proper cleanup - Close file descriptors properly to avoid leaks Co-authored-by: Abanoub Ghadban <AbanoubGhadban@users.noreply.github.com> * Revert process_manager.rb changes from commit 5740b0e - Remove PROCESS_MANAGERS constant (revert to explicit calls) - Revert procfile validation to stricter shell metacharacter check - Keep spec file changes from commit 5740b0e (around hook improvements) Co-authored-by: Abanoub Ghadban <AbanoubGhadban@users.noreply.github.com> * Fix RSpec/ExpectOutput violations by removing global stream reassignment Remove the around hook that was mutating $stdout/$stderr, which triggered RuboCop's RSpec/ExpectOutput warnings. Tests that need to capture output already use RSpec's .to output(...).to_stderr matcher correctly. Co-authored-by: Abanoub Ghadban <AbanoubGhadban@users.noreply.github.com> * Update spec/react_on_rails/dev/process_manager_spec.rb Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * Remove duplicate RSpec.describe for ProcessManager --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abanoub Ghadban <AbanoubGhadban@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 parent 477f17d commit 6c77213

File tree

2 files changed

+306
-40
lines changed

2 files changed

+306
-40
lines changed

lib/react_on_rails/dev/process_manager.rb

Lines changed: 122 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,18 @@
11
# frozen_string_literal: true
22

3+
require "timeout"
4+
35
module ReactOnRails
46
module Dev
57
class ProcessManager
8+
# Timeout for version check operations to prevent hanging
9+
VERSION_CHECK_TIMEOUT = 5
10+
611
class << self
12+
# Check if a process is available and usable in the current execution context
13+
# This accounts for bundler context where system commands might be intercepted
714
def installed?(process)
8-
IO.popen([process, "-v"], &:close)
9-
true
10-
rescue Errno::ENOENT
11-
false
15+
installed_in_current_context?(process)
1216
end
1317

1418
def ensure_procfile(procfile)
@@ -31,20 +35,124 @@ def run_with_process_manager(procfile)
3135
# Clean up stale files before starting
3236
FileManager.cleanup_stale_files
3337

34-
if installed?("overmind")
35-
system("overmind", "start", "-f", procfile)
36-
elsif installed?("foreman")
37-
system("foreman", "start", "-f", procfile)
38+
# Try process managers in order of preference
39+
return if run_process_if_available("overmind", ["start", "-f", procfile])
40+
return if run_process_if_available("foreman", ["start", "-f", procfile])
41+
42+
show_process_manager_installation_help
43+
exit 1
44+
end
45+
46+
private
47+
48+
# Check if a process is actually usable in the current execution context
49+
# This is important for commands that might be intercepted by bundler
50+
def installed_in_current_context?(process)
51+
# Try to execute the process with version flags to see if it works
52+
# Use system() because that's how we'll actually call it later
53+
version_flags_for(process).any? do |flag|
54+
# Add timeout to prevent hanging on version checks
55+
Timeout.timeout(VERSION_CHECK_TIMEOUT) do
56+
system(process, flag, out: File::NULL, err: File::NULL)
57+
end
58+
end
59+
rescue Errno::ENOENT, Timeout::Error
60+
false
61+
end
62+
63+
# Get appropriate version flags for different processes
64+
def version_flags_for(process)
65+
case process
66+
when "overmind"
67+
["--version"]
68+
when "foreman"
69+
["--version", "-v"]
3870
else
39-
warn <<~MSG
40-
NOTICE:
41-
For this script to run, you need either 'overmind' or 'foreman' installed on your machine. Please try this script after installing one of them.
42-
MSG
43-
exit 1
71+
["--version", "-v", "-V"]
4472
end
4573
end
4674

47-
private
75+
# Try to run a process if it's available, with intelligent fallback strategy
76+
# Returns true if process was found and executed, false if not available
77+
def run_process_if_available(process, args)
78+
# First attempt: try in current context (works for bundled processes)
79+
return system(process, *args) if installed?(process)
80+
81+
# Second attempt: try in system context (works for system-installed processes)
82+
return run_process_outside_bundle(process, args) if process_available_in_system?(process)
83+
84+
# Process not available in either context
85+
false
86+
end
87+
88+
# Run a process outside of bundler context
89+
# This allows using system-installed processes even when they're not in the Gemfile
90+
def run_process_outside_bundle(process, args)
91+
if defined?(Bundler)
92+
with_unbundled_context do
93+
system(process, *args)
94+
end
95+
else
96+
# Fallback if Bundler is not available
97+
system(process, *args)
98+
end
99+
end
100+
101+
# Check if a process is available system-wide (outside bundle context)
102+
def process_available_in_system?(process)
103+
return false unless defined?(Bundler)
104+
105+
with_unbundled_context do
106+
# Try version flags to check if process exists outside bundler context
107+
version_flags_for(process).any? do |flag|
108+
# Add timeout to prevent hanging on version checks
109+
Timeout.timeout(VERSION_CHECK_TIMEOUT) do
110+
system(process, flag, out: File::NULL, err: File::NULL)
111+
end
112+
end
113+
end
114+
rescue Errno::ENOENT, Timeout::Error
115+
false
116+
end
117+
118+
# DRY helper method for Bundler context switching with API compatibility
119+
# Supports both new (with_unbundled_env) and legacy (with_clean_env) Bundler APIs
120+
def with_unbundled_context(&block)
121+
if Bundler.respond_to?(:with_unbundled_env)
122+
Bundler.with_unbundled_env(&block)
123+
elsif Bundler.respond_to?(:with_clean_env)
124+
Bundler.with_clean_env(&block)
125+
else
126+
# Fallback if neither method is available (very old Bundler versions)
127+
yield
128+
end
129+
end
130+
131+
# Improved error message with helpful guidance
132+
def show_process_manager_installation_help
133+
warn <<~MSG
134+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
135+
ERROR: Process Manager Not Found
136+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
137+
138+
To run the React on Rails development server, you need either 'overmind' or
139+
'foreman' installed on your system.
140+
141+
RECOMMENDED INSTALLATION:
142+
143+
macOS: brew install overmind
144+
Linux: gem install foreman
145+
146+
IMPORTANT:
147+
DO NOT add foreman to your Gemfile. Install it globally on your system.
148+
149+
For more information about why foreman should not be in your Gemfile, see:
150+
https://github.com/shakacode/react_on_rails/blob/master/docs/javascript/foreman-issues.md
151+
152+
After installation, try running this script again.
153+
━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━
154+
MSG
155+
end
48156

49157
def valid_procfile_path?(procfile)
50158
# Reject paths with shell metacharacters

0 commit comments

Comments
 (0)