Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Rescue SystemCallError instead of some Errnos? #36

Closed
indirect opened this Issue Feb 1, 2013 · 4 comments

Comments

Projects
None yet
2 participants

indirect commented Feb 1, 2013

It seems like persistent.rb:949 wants to rescue SystemCallError instead of Errno::ECONNABORTED, Errno::ECONNRESET, Errno::EPIPE, Errno::EINVAL. The reason I say that is that the latter list is missing at very least Errno::ETIMEDOUT, and probably a few others. So I figure why not just grab the parent exception. :P

Owner

drbrain commented Feb 9, 2013

I prefer to be conservative over rescuing SystemCallError blindly, perhaps I'm too conservative.

I will add Errno::ETIMEDOUT to the list of retriable exceptions.

indirect commented Feb 9, 2013

I feel like it's pretty arguable that if any SystemCallError is raised during a network request, the request failed, and that merits a Net::HTTP::Persistent::Error. Otherwise every client of NHP has to rescue SystemCallError themselves!

Owner

drbrain commented Feb 9, 2013

But not all of Errno::* can be raised by network communication such as Errno::ENOENT or Errno::EACCES. Since #request accepts a block, a non-network SystemCallError may be raised which could lead to incorrect behavior.

indirect commented Feb 9, 2013

Good point. :( That's unfortunate. Thanks for adding ETIMEDOUT!

@indirect indirect closed this Feb 9, 2013

@indirect indirect reopened this Feb 9, 2013

@drbrain drbrain closed this in b772eb9 Feb 9, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment