-
-
Notifications
You must be signed in to change notification settings - Fork 638
Open
Description
Improve bin/dev kill Process Management
Background
In the react_on_rails-demos repository, we recently implemented improved process management for watch processes with proper error handling (see PR #42). This same improvement should be applied to the bin/dev kill
script in react_on_rails.
Problem
The current process killing logic likely has similar issues to what we found in the demos repo:
- Counting processes as "killed" even when
Process.kill
fails due to permission errors (EPERM) - Not distinguishing between successful kills, already-stopped processes, and permission-denied errors
- Potentially confusing user feedback about what actually happened
Solution
Apply the same improved error handling pattern that was implemented in react_on_rails-demos:
Current Pattern (Incorrect)
begin
Process.kill('TERM', pid)
rescue Errno::ESRCH, Errno::EPERM
# Treats both cases the same
end
killed_count += 1 # Always increments, even on EPERM!
Improved Pattern
begin
Process.kill('TERM', pid)
killed_count += 1
rescue Errno::ESRCH
# Process already stopped - treat as success
puts "Process #{pid} - process no longer exists"
killed_count += 1
rescue Errno::EPERM
# Permission denied - do NOT count as success
puts "⚠️ Process #{pid} - permission denied (process owned by another user)"
end
Key Improvements
- Accurate counting: Only increment
killed_count
when kill succeeds or process is already gone (ESRCH) - Clear feedback: Distinguish between:
- ✅ Successfully killed
- ✅ Already stopped (ESRCH - treat as success)
⚠️ Permission denied (EPERM - NOT a success)
- Better UX: Users know exactly what happened with each process
Reference Implementation
See the complete implementation in react_on_rails-demos:
- File:
lib/demo_scripts/gem_swapper.rb
lines 95-118 - PR: Add watch mode process management for swap-deps (#37) react_on_rails-demos#42
- Commit: shakacode/react_on_rails-demos@04702fc
Testing Checklist
When implementing this fix:
- Test killing processes you own (should succeed)
- Test with already-stopped processes (should handle ESRCH gracefully)
- Test with processes owned by other users (should show EPERM warning and not count as success)
- Verify final count matches actual successful kills
Related
This is part of broader improvements to process management that were implemented in the demos repository for issue #37.
Metadata
Metadata
Assignees
Labels
No labels