Skip to content

Commit

Permalink
Catch exceptions in run_hooks mehtod
Browse files Browse the repository at this point in the history
  • Loading branch information
andrykonchin committed Mar 8, 2020
1 parent 8001792 commit 262331f
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 8 deletions.
10 changes: 5 additions & 5 deletions lib/puma/cluster.rb
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def spawn_workers

diff.times do
idx = next_worker_index
@launcher.config.run_hooks :before_worker_fork, idx
@launcher.config.run_hooks :before_worker_fork, idx, @launcher.events

pid = fork { worker(idx, master) }
if !pid
Expand All @@ -149,7 +149,7 @@ def spawn_workers
debug "Spawned worker: #{pid}"
@workers << Worker.new(idx, pid, @phase, @options)

@launcher.config.run_hooks :after_worker_fork, idx
@launcher.config.run_hooks :after_worker_fork, idx, @launcher.events
end

if diff > 0
Expand Down Expand Up @@ -268,7 +268,7 @@ def worker(index, master)

# Invoke any worker boot hooks so they can get
# things in shape before booting the app.
@launcher.config.run_hooks :before_worker_boot, index
@launcher.config.run_hooks :before_worker_boot, index, @launcher.events

server = start_server

Expand Down Expand Up @@ -310,7 +310,7 @@ def worker(index, master)

# Invoke any worker shutdown hooks so they can prevent the worker
# exiting until any background operations are completed
@launcher.config.run_hooks :before_worker_shutdown, index
@launcher.config.run_hooks :before_worker_shutdown, index, @launcher.events
ensure
@worker_write << "t#{Process.pid}\n" rescue nil
@worker_write.close
Expand Down Expand Up @@ -486,7 +486,7 @@ def run

@master_read, @worker_write = read, @wakeup

@launcher.config.run_hooks :before_fork, nil
@launcher.config.run_hooks :before_fork, nil, @launcher.events
GC.compact if GC.respond_to?(:compact)

spawn_workers
Expand Down
11 changes: 9 additions & 2 deletions lib/puma/configuration.rb
Original file line number Diff line number Diff line change
Expand Up @@ -275,8 +275,15 @@ def load_plugin(name)
@plugins.create name
end

def run_hooks(key, arg)
@options.all_of(key).each { |b| b.call arg }
def run_hooks(key, arg, events)
@options.all_of(key).each do |b|
begin
b.call arg
rescue => e
events.log "WARNING hook #{key} failed with exception (#{e.class}) #{e.message}"
events.debug e.full_message
end
end
end

def self.temp_path
Expand Down
2 changes: 1 addition & 1 deletion lib/puma/launcher.rb
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ def reload_worker_directory
end

def restart!
@config.run_hooks :on_restart, self
@config.run_hooks :on_restart, self, @events

if Puma.jruby?
close_binder_listeners
Expand Down
16 changes: 16 additions & 0 deletions test/test_config.rb
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,22 @@ def test_config_raise_exception_on_sigterm
conf.options[:raise_exception_on_sigterm] = true
assert_equal conf.options[:raise_exception_on_sigterm], true
end

def test_run_hooks_and_exception
require 'puma/events'

conf = Puma::Configuration.new do |c|
c.on_restart do |a|
raise RuntimeError, 'Error from hook'
end
end
conf.load
events = Puma::Events.strings

conf.run_hooks :on_restart, 'ARG', events
expected = /WARNING hook on_restart failed with exception \(RuntimeError\) Error from hook/
assert_match expected, events.stdout.string
end
end

# Thread unsafe modification of ENV
Expand Down

0 comments on commit 262331f

Please sign in to comment.