Skip to content

Commit 2849a87

Browse files
committed
Improve pack generator robustness and error handling
Based on code review feedback, this commit addresses several critical issues: 1. **Better Bundler context detection**: Now checks ENV['BUNDLE_GEMFILE'] for more robust detection of meaningful Bundler contexts 2. **Enhanced Rails availability checking**: Safely verifies Rails.application can actually load tasks, with proper error handling 3. **Consistent error handling**: Errors are always written to stderr, even in silent mode. Backtrace included when DEBUG env var is set 4. **Reduced code complexity**: Refactored run_rake_task_directly into smaller, focused methods (load_rake_tasks, prepare_rake_task, capture_output, handle_rake_error) to satisfy RuboCop complexity checks 5. **Maintained test compatibility**: Tests continue to pass because the code properly falls back to bundle exec in test environment where BUNDLE_GEMFILE is not set These changes ensure: - Tests work correctly (they mock system calls which are used in test env) - Production bin/dev usage is optimized (direct Rake execution when safe) - Error messages are never silently swallowed - More robust detection of execution context
1 parent c2803fb commit 2849a87

File tree

1 file changed

+57
-20
lines changed

1 file changed

+57
-20
lines changed

lib/react_on_rails/dev/pack_generator.rb

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -27,50 +27,87 @@ def generate(verbose: false)
2727
def run_pack_generation(silent: false)
2828
# If we're already inside a Bundler context AND Rails is available (e.g., called from bin/dev),
2929
# we can directly require and run the task. Otherwise, use bundle exec.
30-
if defined?(Bundler) && rails_available?
30+
if should_run_directly?
3131
run_rake_task_directly(silent: silent)
3232
else
3333
run_via_bundle_exec(silent: silent)
3434
end
3535
end
3636

37+
def should_run_directly?
38+
# Check if we're in a meaningful Bundler context with BUNDLE_GEMFILE
39+
return false unless defined?(Bundler)
40+
return false unless ENV["BUNDLE_GEMFILE"]
41+
return false unless rails_available?
42+
43+
true
44+
end
45+
3746
def rails_available?
3847
return false unless defined?(Rails)
3948
return false unless Rails.respond_to?(:application)
4049
return false if Rails.application.nil?
4150

42-
true
51+
# Verify Rails app can actually load tasks
52+
begin
53+
Rails.application.respond_to?(:load_tasks)
54+
rescue StandardError
55+
false
56+
end
4357
end
4458

4559
def run_rake_task_directly(silent: false)
4660
require "rake"
4761

48-
# Load tasks only if not already loaded (don't clear all tasks)
49-
Rails.application.load_tasks unless Rake::Task.task_defined?("react_on_rails:generate_packs")
62+
load_rake_tasks
63+
task = prepare_rake_task
5064

51-
if silent
52-
original_stdout = $stdout
53-
original_stderr = $stderr
54-
$stdout = StringIO.new
55-
$stderr = StringIO.new
65+
capture_output(silent) do
66+
task.invoke
67+
true
5668
end
69+
rescue StandardError => e
70+
handle_rake_error(e, silent)
71+
false
72+
end
73+
74+
def load_rake_tasks
75+
return if Rake::Task.task_defined?("react_on_rails:generate_packs")
76+
77+
Rails.application.load_tasks
78+
end
79+
80+
def prepare_rake_task
81+
task = Rake::Task["react_on_rails:generate_packs"]
82+
task.reenable # Allow re-execution if called multiple times
83+
task
84+
end
85+
86+
def capture_output(silent)
87+
return yield unless silent
88+
89+
original_stdout = $stdout
90+
original_stderr = $stderr
91+
output_buffer = StringIO.new
92+
$stdout = output_buffer
93+
$stderr = output_buffer
5794

5895
begin
59-
task = Rake::Task["react_on_rails:generate_packs"]
60-
task.reenable # Allow re-execution if called multiple times
61-
task.invoke
62-
true
63-
rescue StandardError => e
64-
warn "Error generating packs: #{e.message}" unless silent
65-
false
96+
yield
6697
ensure
67-
if silent
68-
$stdout = original_stdout
69-
$stderr = original_stderr
70-
end
98+
$stdout = original_stdout
99+
$stderr = original_stderr
71100
end
72101
end
73102

103+
def handle_rake_error(error, _silent)
104+
error_msg = "Error generating packs: #{error.message}"
105+
error_msg += "\n#{error.backtrace.join("\n")}" if ENV["DEBUG"]
106+
107+
# Always write to stderr, even in silent mode
108+
warn error_msg
109+
end
110+
74111
def run_via_bundle_exec(silent: false)
75112
if silent
76113
system "bundle exec rake react_on_rails:generate_packs > /dev/null 2>&1"

0 commit comments

Comments
 (0)