[WIP] Refactor: Split out LogWriter from Events (no logic change) (pu…

…ma#2798)

* Split out LogWriter from Events

* Improve code comment

* Fix constructor interfaces

* Fix file includes

* Fix specs and requires

* Fix LogWriter

* More fixes

* Fix tests

* Fix specs

* Fix spec

* Fix more specs

* Refactor: Split out LogWriter from Events

* Improve comments

* Fix bundle pruner

Co-authored-by: shields <shields@tablecheck.com>

examples/puma/client-certs/run_server_with_certs.rb

@@ -8,8 +8,8 @@
   p env['puma.peercert']
   [200, {}, [ env['puma.peercert'] ]]
 }
-events = Puma::Events.new($stdout, $stderr)
-server = Puma::Server.new(app, events)
+log_writer = Puma::LogWriter.new($stdout, $stderr)
+server = Puma::Server.new(app, log_writer)
 
 context = Puma::MiniSSL::Context.new
 context.key         = "certs/server.key"

lib/puma/binder.rb

@@ -28,8 +28,8 @@ class Binder
 
     RACK_VERSION = [1,6].freeze
 
-    def initialize(events, conf = Configuration.new)
-      @events = events
+    def initialize(log_writer, conf = Configuration.new)
+      @log_writer = log_writer
       @conf = conf
       @listeners = []
       @inherited_fds = {}
@@ -38,7 +38,7 @@ def initialize(events, conf = Configuration.new)
 
       @proto_env = {
         "rack.version".freeze => RACK_VERSION,
-        "rack.errors".freeze => events.stderr,
+        "rack.errors".freeze => log_writer.stderr,
         "rack.multithread".freeze => conf.options[:max_threads] > 1,
         "rack.multiprocess".freeze => conf.options[:workers] >= 1,
         "rack.run_once".freeze => false,
@@ -98,7 +98,7 @@ def create_inherited_fds(env_hash)
     # @version 5.0.0
     #
     def create_activated_fds(env_hash)
-      @events.debug "ENV['LISTEN_FDS'] #{ENV['LISTEN_FDS'].inspect}  env_hash['LISTEN_PID'] #{env_hash['LISTEN_PID'].inspect}"
+      @log_writer.debug "ENV['LISTEN_FDS'] #{ENV['LISTEN_FDS'].inspect}  env_hash['LISTEN_PID'] #{env_hash['LISTEN_PID'].inspect}"
       return [] unless env_hash['LISTEN_FDS'] && env_hash['LISTEN_PID'].to_i == $$
       env_hash['LISTEN_FDS'].to_i.times do |index|
         sock = TCPServer.for_fd(socket_activation_fd(index))
@@ -110,7 +110,7 @@ def create_activated_fds(env_hash)
           [:tcp, addr, port]
         end
         @activated_sockets[key] = sock
-        @events.debug "Registered #{key.join ':'} for activation from LISTEN_FDS"
+        @log_writer.debug "Registered #{key.join ':'} for activation from LISTEN_FDS"
       end
       ["LISTEN_FDS", "LISTEN_PID"] # Signal to remove these keys from ENV
     end
@@ -152,17 +152,17 @@ def synthesize_binds_from_activated_fs(binds, only_matching)
       end
     end
 
-    def parse(binds, logger, log_msg = 'Listening')
+    def parse(binds, log_writer, log_msg = 'Listening')
       binds.each do |str|
         uri = URI.parse str
         case uri.scheme
         when "tcp"
           if fd = @inherited_fds.delete(str)
             io = inherit_tcp_listener uri.host, uri.port, fd
-            logger.log "* Inherited #{str}"
+            log_writer.log "* Inherited #{str}"
           elsif sock = @activated_sockets.delete([ :tcp, uri.host, uri.port ])
             io = inherit_tcp_listener uri.host, uri.port, sock
-            logger.log "* Activated #{str}"
+            log_writer.log "* Activated #{str}"
           else
             ios_len = @ios.length
             params = Util.parse_query uri.query
@@ -174,7 +174,7 @@ def parse(binds, logger, log_msg = 'Listening')
 
             @ios[ios_len..-1].each do |i|
               addr = loc_addr_str i
-              logger.log "* #{log_msg} on http://#{addr}"
+              log_writer.log "* #{log_msg} on http://#{addr}"
             end
           end
 
@@ -191,12 +191,12 @@ def parse(binds, logger, log_msg = 'Listening')
           if fd = @inherited_fds.delete(str)
             @unix_paths << path unless abstract
             io = inherit_unix_listener path, fd
-            logger.log "* Inherited #{str}"
+            log_writer.log "* Inherited #{str}"
           elsif sock = @activated_sockets.delete([ :unix, path ]) ||
               @activated_sockets.delete([ :unix, File.realdirpath(path) ])
             @unix_paths << path unless abstract || File.exist?(path)
             io = inherit_unix_listener path, sock
-            logger.log "* Activated #{str}"
+            log_writer.log "* Activated #{str}"
           else
             umask = nil
             mode = nil
@@ -220,7 +220,7 @@ def parse(binds, logger, log_msg = 'Listening')
 
             @unix_paths << path unless abstract || File.exist?(path)
             io = add_unix_listener path, umask, mode, backlog
-            logger.log "* #{log_msg} on #{str}"
+            log_writer.log "* #{log_msg} on #{str}"
           end
 
           @listeners << [str, io]
@@ -246,36 +246,36 @@ def parse(binds, logger, log_msg = 'Listening')
                   params["#{v}_pem"] = @conf.options[:store][index]
                 end
               end
-              MiniSSL::ContextBuilder.new(params, @events).context
+              MiniSSL::ContextBuilder.new(params, @log_writer).context
             end
 
           if fd = @inherited_fds.delete(str)
-            logger.log "* Inherited #{str}"
+            log_writer.log "* Inherited #{str}"
             io = inherit_ssl_listener fd, ctx
           elsif sock = @activated_sockets.delete([ :tcp, uri.host, uri.port ])
             io = inherit_ssl_listener sock, ctx
-            logger.log "* Activated #{str}"
+            log_writer.log "* Activated #{str}"
           else
             ios_len = @ios.length
             backlog = params.fetch('backlog', 1024).to_i
             io = add_ssl_listener uri.host, uri.port, ctx, optimize_for_latency = true, backlog
 
             @ios[ios_len..-1].each do |i|
               addr = loc_addr_str i
-              logger.log "* #{log_msg} on ssl://#{addr}?#{uri.query}"
+              log_writer.log "* #{log_msg} on ssl://#{addr}?#{uri.query}"
             end
           end
 
           @listeners << [str, io] if io
         else
-          logger.error "Invalid URI: #{str}"
+          log_writer.error "Invalid URI: #{str}"
         end
       end
 
       # If we inherited fds but didn't use them (because of a
       # configuration change), then be sure to close them.
       @inherited_fds.each do |str, fd|
-        logger.log "* Closing unused inherited connection: #{str}"
+        log_writer.log "* Closing unused inherited connection: #{str}"
 
         begin
           IO.for_fd(fd).close
@@ -295,7 +295,7 @@ def parse(binds, logger, log_msg = 'Listening')
         fds = @ios.map(&:to_i)
         @activated_sockets.each do |key, sock|
           next if fds.include? sock.to_i
-          logger.log "* Closing unused activated socket: #{key.first}://#{key[1..-1].join ':'}"
+          log_writer.log "* Closing unused activated socket: #{key.first}://#{key[1..-1].join ':'}"
           begin
             sock.close
           rescue SystemCallError
@@ -319,7 +319,7 @@ def localhost_authority_context
         local_certificates_path = File.expand_path("~/.localhost")
         [File.join(local_certificates_path, "localhost.key"), File.join(local_certificates_path, "localhost.crt")]
       end
-      MiniSSL::ContextBuilder.new({ "key" => key_path, "cert" => crt_path }, @events).context
+      MiniSSL::ContextBuilder.new({ "key" => key_path, "cert" => crt_path }, @log_writer).context
     end
 
     # Tell the server to listen on host +host+, port +port+.

lib/puma/cli.rb

@@ -7,7 +7,7 @@
 require 'puma/configuration'
 require 'puma/launcher'
 require 'puma/const'
-require 'puma/events'
+require 'puma/log_writer'
 
 module Puma
   class << self
@@ -30,10 +30,10 @@ class CLI
     # +stdout+ and +stderr+ can be set to IO-like objects which
     # this object will report status on.
     #
-    def initialize(argv, events=Events.stdio)
+    def initialize(argv, log_writer = LogWriter.stdio, events = Events.new)
       @debug = false
       @argv = argv.dup
-
+      @log_writer = log_writer
       @events = events
 
       @conf = nil
@@ -69,7 +69,7 @@ def initialize(argv, events=Events.stdio)
         end
       end
 
-      @launcher = Puma::Launcher.new(@conf, :events => @events, :argv => argv)
+      @launcher = Puma::Launcher.new(@conf, :log_writer => @log_writer, :events => @events, :argv => argv)
     end
 
     attr_reader :launcher
@@ -83,7 +83,7 @@ def run
 
     private
     def unsupported(str)
-      @events.error(str)
+      @log_writer.error(str)
       raise UnsupportedOption
     end
 
@@ -186,7 +186,7 @@ def setup_options
           end
 
           o.on "-s", "--silent", "Do not log prompt messages other than errors" do
-            @events = Events.new NullIO.new, $stderr
+            @log_writer = LogWriter.new(NullIO.new, $stderr)
           end
 
           o.on "-S", "--state PATH", "Where to store the state details" do |arg|

lib/puma/cluster.rb

@@ -17,8 +17,8 @@ module Puma
   # via the `spawn_workers` method call. Each worker will have it's own
   # instance of a `Puma::Server`.
   class Cluster < Runner
-    def initialize(cli, events)
-      super cli, events
+    def initialize(launcher)
+      super(launcher)
 
       @phase = 0
       @workers = []
@@ -96,7 +96,7 @@ def spawn_workers
 
     # @version 5.0.0
     def spawn_worker(idx, master)
-      @launcher.config.run_hooks :before_worker_fork, idx, @launcher.events
+      @launcher.config.run_hooks(:before_worker_fork, idx, @launcher.log_writer)
 
       pid = fork { worker(idx, master) }
       if !pid
@@ -105,7 +105,7 @@ def spawn_worker(idx, master)
         exit! 1
       end
 
-      @launcher.config.run_hooks :after_worker_fork, idx, @launcher.events
+      @launcher.config.run_hooks(:after_worker_fork, idx, @launcher.log_writer)
       pid
     end
 
@@ -413,8 +413,8 @@ def run
 
       @master_read, @worker_write = read, @wakeup
 
-      @launcher.config.run_hooks :before_fork, nil, @launcher.events
-      Puma::Util.nakayoshi_gc @events if @options[:nakayoshi_fork]
+      @launcher.config.run_hooks(:before_fork, nil, @launcher.log_writer)
+      Puma::Util.nakayoshi_gc(@log_writer) if @options[:nakayoshi_fork]
 
       spawn_workers
 

lib/puma/cluster/worker.rb

@@ -12,7 +12,7 @@ class Worker < Puma::Runner
       attr_reader :index, :master
 
       def initialize(index:, master:, launcher:, pipes:, server: nil)
-        super launcher, launcher.events
+        super(launcher)
 
         @index = index
         @master = master
@@ -52,7 +52,7 @@ def run
 
         # 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.events
+        @launcher.config.run_hooks(:before_worker_boot, index, @launcher.log_writer)
 
         begin
         server = @server ||= start_server
@@ -83,8 +83,8 @@ def run
                 if restart_server.length > 0
                   restart_server.clear
                   server.begin_restart(true)
-                  @launcher.config.run_hooks :before_refork, nil, @launcher.events
-                  Puma::Util.nakayoshi_gc @events if @options[:nakayoshi_fork]
+                  @launcher.config.run_hooks(:before_refork, nil, @launcher.log_writer)
+                  Puma::Util.nakayoshi_gc(@log_writer) if @options[:nakayoshi_fork]
                 end
               elsif idx == 0 # restart server
                 restart_server << true << false
@@ -138,7 +138,7 @@ def run
 
         # 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.events
+        @launcher.config.run_hooks(:before_worker_shutdown, index, @launcher.log_writer)
       ensure
         @worker_write << "t#{Process.pid}\n" rescue nil
         @worker_write.close
@@ -147,7 +147,7 @@ def run
       private
 
       def spawn_worker(idx)
-        @launcher.config.run_hooks :before_worker_fork, idx, @launcher.events
+        @launcher.config.run_hooks(:before_worker_fork, idx, @launcher.log_writer)
 
         pid = fork do
           new_worker = Worker.new index: idx,
@@ -165,7 +165,7 @@ def spawn_worker(idx)
           exit! 1
         end
 
-        @launcher.config.run_hooks :after_worker_fork, idx, @launcher.events
+        @launcher.config.run_hooks(:after_worker_fork, idx, @launcher.log_writer)
         pid
       end
     end

lib/puma/configuration.rb

@@ -291,13 +291,13 @@ def load_plugin(name)
       @plugins.create name
     end
 
-    def run_hooks(key, arg, events)
+    def run_hooks(key, arg, log_writer)
       @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.backtrace.join("\n")
+          log_writer.log "WARNING hook #{key} failed with exception (#{e.class}) #{e.message}"
+          log_writer.debug e.backtrace.join("\n")
         end
       end
     end

lib/puma/control_cli.rb

@@ -293,13 +293,13 @@ def start
       run_args += ["-C", @config_file] if @config_file
       run_args += ["-e", @environment] if @environment
 
-      events = Puma::Events.new @stdout, @stderr
+      log_writer = Puma::LogWriter.new(@stdout, @stderr)
 
       # replace $0 because puma use it to generate restart command
       puma_cmd = $0.gsub(/pumactl$/, 'puma')
       $0 = puma_cmd if File.exist?(puma_cmd)
 
-      cli = Puma::CLI.new run_args, events
+      cli = Puma::CLI.new run_args, log_writer
       cli.run
     end
   end