Skip to content

Commit ad9a44f

Browse files
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 <[email protected]>
1 parent d63cee8 commit ad9a44f

File tree

2 files changed

+32
-6
lines changed

2 files changed

+32
-6
lines changed

lib/react_on_rails/dev/process_manager.rb

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# frozen_string_literal: true
22

3+
require "timeout"
4+
35
module ReactOnRails
46
module Dev
57
class ProcessManager
@@ -46,9 +48,12 @@ def installed_in_current_context?(process)
4648
# Try to execute the process with version flags to see if it works
4749
# Use system() because that's how we'll actually call it later
4850
version_flags_for(process).any? do |flag|
49-
system(process, flag, out: File::NULL, err: File::NULL)
51+
# Add timeout to prevent hanging on version checks
52+
Timeout.timeout(5) do
53+
system(process, flag, out: File::NULL, err: File::NULL)
54+
end
5055
end
51-
rescue Errno::ENOENT
56+
rescue Errno::ENOENT, Timeout::Error
5257
false
5358
end
5459

@@ -97,10 +102,13 @@ def process_available_in_system?(process)
97102
Bundler.with_unbundled_env do
98103
# Try version flags to check if process exists outside bundler context
99104
version_flags_for(process).any? do |flag|
100-
system(process, flag, out: File::NULL, err: File::NULL)
105+
# Add timeout to prevent hanging on version checks
106+
Timeout.timeout(5) do
107+
system(process, flag, out: File::NULL, err: File::NULL)
108+
end
101109
end
102110
end
103-
rescue Errno::ENOENT
111+
rescue Errno::ENOENT, Timeout::Error
104112
false
105113
end
106114

spec/react_on_rails/dev/process_manager_spec.rb

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,19 @@
1919

2020
describe ".installed?" do
2121
it "returns true when process is available in current context" do
22+
expect(Timeout).to receive(:timeout).with(5).and_yield
2223
expect_any_instance_of(Kernel).to receive(:system)
2324
.with("overmind", "--version", out: File::NULL, err: File::NULL).and_return(true)
2425
expect(described_class).to be_installed("overmind")
2526
end
2627

2728
it "returns false when process is not available in current context" do
28-
expect_any_instance_of(Kernel).to receive(:system)
29-
.with("nonexistent", "--version", out: File::NULL, err: File::NULL).and_raise(Errno::ENOENT)
29+
expect(Timeout).to receive(:timeout).with(5).and_raise(Errno::ENOENT)
3030
expect(described_class.installed?("nonexistent")).to be false
3131
end
3232

3333
it "returns false when all version flags fail" do
34+
expect(Timeout).to receive(:timeout).with(5).exactly(3).times.and_yield
3435
expect_any_instance_of(Kernel).to receive(:system)
3536
.with("failing_process", "--version", out: File::NULL, err: File::NULL).and_return(false)
3637
expect_any_instance_of(Kernel).to receive(:system)
@@ -41,12 +42,18 @@
4142
end
4243

4344
it "returns true when second version flag succeeds" do
45+
expect(Timeout).to receive(:timeout).with(5).twice.and_yield
4446
expect_any_instance_of(Kernel).to receive(:system)
4547
.with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(false)
4648
expect_any_instance_of(Kernel).to receive(:system)
4749
.with("foreman", "-v", out: File::NULL, err: File::NULL).and_return(true)
4850
expect(described_class.installed?("foreman")).to be true
4951
end
52+
53+
it "returns false when version check times out" do
54+
expect(Timeout).to receive(:timeout).with(5).and_raise(Timeout::Error)
55+
expect(described_class.installed?("hanging_process")).to be false
56+
end
5057
end
5158

5259
describe ".ensure_procfile" do
@@ -155,6 +162,7 @@
155162
bundler_double = class_double(Bundler)
156163
stub_const("Bundler", bundler_double)
157164
expect(bundler_double).to receive(:with_unbundled_env).and_yield
165+
expect(Timeout).to receive(:timeout).with(5).and_yield
158166
expect_any_instance_of(Kernel).to receive(:system)
159167
.with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(true)
160168

@@ -171,13 +179,23 @@
171179
bundler_double = class_double(Bundler)
172180
stub_const("Bundler", bundler_double)
173181
expect(bundler_double).to receive(:with_unbundled_env).and_yield
182+
expect(Timeout).to receive(:timeout).with(5).twice.and_yield
174183
expect_any_instance_of(Kernel).to receive(:system)
175184
.with("foreman", "--version", out: File::NULL, err: File::NULL).and_return(false)
176185
expect_any_instance_of(Kernel).to receive(:system)
177186
.with("foreman", "-v", out: File::NULL, err: File::NULL).and_return(true)
178187

179188
expect(described_class.send(:process_available_in_system?, "foreman")).to be true
180189
end
190+
191+
it "returns false when version check times out in system context" do
192+
bundler_double = class_double(Bundler)
193+
stub_const("Bundler", bundler_double)
194+
expect(bundler_double).to receive(:with_unbundled_env).and_yield
195+
expect(Timeout).to receive(:timeout).with(5).and_raise(Timeout::Error)
196+
197+
expect(described_class.send(:process_available_in_system?, "hanging_process")).to be false
198+
end
181199
end
182200

183201
describe ".version_flags_for" do

0 commit comments

Comments
 (0)