EPIPE error not handled like an IOError

[micahhainlinestitchfix]

Describe the bug
In a large production system, we get an error on write that looks like it should be caught by the existing puma error handling, but isn't.

In server.rb there's a piece of code that looks like this:

   def notify_safely(message)
      begin
        @notify << message
      rescue IOError
         # The server, in another thread, is shutting down
        Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
      rescue RuntimeError => e
        # Temporary workaround for https://bugs.ruby-lang.org/issues/13239
        if e.message.include?('IOError')
          Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
        else
          raise e
        end
      end
    end

When the message fails with an Errno::EPIPE broken pipe, it looks like that should fall under the rescue, but as IOError says

# Note that some IO failures raise <code>SystemCallError</code>s
# and these are not subclasses of IOError:

Since the intention seems to be to rescue the IO errors and do proper cleanup, it seems reasonable to include a rescue SystemCallError => e to handle the set of IO error that are thrown that way.

Puma config:

workers 2
threads_count = 1
threads threads_count, threads_count

preload_app!

rackup      DefaultRackup
port        ENV["PORT"]     || 3000
environment ENV["RACK_ENV"] || "development"
queue_requests(false)

plugin :puma_stats

on_worker_boot do
  APP_DATABASES.each(&:connect!)
end

CMD ["sh", "-c", "exec bundle exec puma -C config/puma.rb"]

To Reproduce
This runs in a large complex system, I don't have an example project that has a broken pipe.

Expected behavior

Desktop (please complete the following information):

• OS: Linux
• Puma Version 4.3.5

[dentarg]

Sounds like what was fixed in #2312, see https://github.com/puma/puma/pull/2312/files#diff-ef4c610f0ce605eb90f2810856394fd5

Code now reads

puma/lib/puma/server.rb

Lines 916 to 932 in 0f718d5

	def notify_safely(message)
	@check, @notify = Puma::Util.pipe unless @notify
	begin
	@notify << message
	rescue IOError, NoMethodError, Errno::EPIPE
	# The server, in another thread, is shutting down
	Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
	rescue RuntimeError => e
	# Temporary workaround for https://bugs.ruby-lang.org/issues/13239
	if e.message.include?('IOError')
	Thread.current.purge_interrupt_queue if Thread.current.respond_to? :purge_interrupt_queue
	else
	raise e
	end
	end
	end
	private :notify_safely

@micahhainlinestitchfix are you able to try out the master branch in your environment?

[micahhainlinestitchfix]

Awesome! I don't think it would be a good idea to run master in my environment, but I'm very glad to see the issue is addressed.

I'll look into upgrading to the latest release when available!

[micahhainlinestitchfix]

I guess I still have a question if this change is scheduled for release in the next 4.x.x release?

[MSP-Greg]

@micahhainlinestitchfix

Thanks for the report. Some of the ways Puma and/or sockets may error are really difficult to duplicate in CI, especially errors that intermittently occur in loaded production servers. Re fixes for them, making rescue clauses overly broad may hide errors that should be thrown...

I still have a question if this change is scheduled for release in the next 4.x.x release?

Good question. Rephrasing: 'Should 4.x be supported (bug fixes only) and if so, for how long?'

[micahhainlinestitchfix]

@MSP-Greg

Good question. Rephrasing: 'Should 4.x be supported (bug fixes only) and if so, for how long?'

It is the latest stable release, I'm not sure what else would happen. Are you talking about a stable 5.0 release as the alternative?

[MSP-Greg]

@micahhainlinestitchfix

Sorry, I wasn't clear. Several recent issues/PR's are based on 4.x, and 5.0 doesn't (yet) have a stable release. If it did, I don't know if most users would update to 5.0, or stick with 4.x for some time period.

So, my comment was really meant as a question, not a statement. Maybe I should have added 'If 5.0.0 is released, what percentage of people will adopt it immediately?'

[dentarg]

@micahhainlinestitchfix From following the Puma project for a while now, I've gotten the feeling that the next release is 5.0, so I wouldn't expect a 4.x release with this fix. Puma 5.0 will fix many other bugs, see the History file and the Upgrade file.

[micahhainlinestitchfix]

Awesome! I'll be pushing for fast 5.0 adoption when it's out then. Thanks for the help and information as well as the hard work keeping Puma moving forward!