Skip to content

Commit 5f195a7

Browse files
justin808claude
andauthored
Improve bin/dev kill error handling (#1858) (#1859)
* Improve bin/dev kill error handling for process termination Enhance the terminate_processes method to distinguish between ESRCH (process already stopped) and EPERM (permission denied) errors, providing clearer feedback to users when process termination fails. Key improvements: - Separate handling for ESRCH vs EPERM exceptions - User warning when permission is denied for a process - Clearer indication of what actually happened during process termination This change aligns with the pattern implemented in react_on_rails-demos PR #42 for better process management. Fixes #1858 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> * Add comprehensive test coverage and edge case handling for terminate_processes Improvements based on PR feedback: 1. Test Coverage: - Add 7 new test cases for terminate_processes method - Cover ESRCH, EPERM, ArgumentError, RangeError scenarios - Test mixed success/error cases 2. Edge Case Handling: - Handle ArgumentError (invalid signal) - Handle RangeError (invalid PID) - Consistent with file_manager.rb patterns 3. Return Value Consistency: - All rescue branches now explicitly return nil - Consistent behavior across all error types 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 24496cd commit 5f195a7

File tree

2 files changed

+66
-1
lines changed

2 files changed

+66
-1
lines changed

lib/react_on_rails/dev/server_manager.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,12 @@ def find_process_pids(pattern)
7070
def terminate_processes(pids)
7171
pids.each do |pid|
7272
Process.kill("TERM", pid)
73-
rescue StandardError
73+
rescue Errno::ESRCH, ArgumentError, RangeError
74+
# Process already stopped, or invalid signal/PID - silently skip
75+
nil
76+
rescue Errno::EPERM
77+
# Permission denied - warn the user
78+
puts " ⚠️ Process #{pid} - permission denied (process owned by another user)"
7479
nil
7580
end
7681
end

spec/react_on_rails/dev/server_manager_spec.rb

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -203,6 +203,66 @@ def mock_system_calls
203203
end
204204
end
205205

206+
describe ".terminate_processes" do
207+
it "successfully kills processes" do
208+
pids = [1234, 5678]
209+
expect(Process).to receive(:kill).with("TERM", 1234)
210+
expect(Process).to receive(:kill).with("TERM", 5678)
211+
212+
described_class.terminate_processes(pids)
213+
end
214+
215+
it "handles ESRCH (process not found) silently" do
216+
pids = [1234]
217+
allow(Process).to receive(:kill).with("TERM", 1234).and_raise(Errno::ESRCH)
218+
219+
# Should not raise an error and should not output anything
220+
expect { described_class.terminate_processes(pids) }.not_to raise_error
221+
end
222+
223+
it "handles EPERM (permission denied) with warning" do
224+
pids = [1234]
225+
allow(Process).to receive(:kill).with("TERM", 1234).and_raise(Errno::EPERM)
226+
227+
# Should not raise an error but should output a warning
228+
expect { described_class.terminate_processes(pids) }.to output(/permission denied/).to_stdout_from_any_process
229+
end
230+
231+
it "handles mixed success and ESRCH" do
232+
pids = [1234, 5678]
233+
expect(Process).to receive(:kill).with("TERM", 1234)
234+
allow(Process).to receive(:kill).with("TERM", 5678).and_raise(Errno::ESRCH)
235+
236+
expect { described_class.terminate_processes(pids) }.not_to raise_error
237+
end
238+
239+
it "handles mixed success and EPERM" do
240+
pids = [1234, 5678]
241+
expect(Process).to receive(:kill).with("TERM", 1234)
242+
allow(Process).to receive(:kill).with("TERM", 5678).and_raise(Errno::EPERM)
243+
244+
expect do
245+
described_class.terminate_processes(pids)
246+
end.to output(/5678.*permission denied/).to_stdout_from_any_process
247+
end
248+
249+
it "handles ArgumentError (invalid signal)" do
250+
pids = [1234]
251+
allow(Process).to receive(:kill).with("TERM", 1234).and_raise(ArgumentError)
252+
253+
# Should not raise an error
254+
expect { described_class.terminate_processes(pids) }.not_to raise_error
255+
end
256+
257+
it "handles RangeError (invalid PID)" do
258+
pids = [999_999_999_999]
259+
allow(Process).to receive(:kill).with("TERM", 999_999_999_999).and_raise(RangeError)
260+
261+
# Should not raise an error
262+
expect { described_class.terminate_processes(pids) }.not_to raise_error
263+
end
264+
end
265+
206266
describe ".show_help" do
207267
it "displays help information" do
208268
expect { described_class.show_help }.to output(%r{Usage: bin/dev \[command\]}).to_stdout_from_any_process

0 commit comments

Comments
 (0)