8.1 breaks Celluloid::IO compatibility?

[zuk]

The following code works in 7.x but seems to be broken in 8.1:

require 'celluloid/io'
require 'http'

class Foo
  include Celluloid::IO

  def req
    HTTP
    .get("https://www.google.com", ssl_socket_class: Celluloid::IO::SSLSocket)
  end
end

foo = Foo.new
future = foo.future.req

puts future.value

Running this under 8.1 gets me:

.../ruby-2.1.5/gems/http-0.8.1/lib/http/timeout/null.rb:29:in `start_tls': undefined method `sync_close=' for #<Celluloid::IO::SSLSocket:0x007fcdbce990d0> (NoMethodError)
    from .../ruby-2.1.5/gems/http-0.8.1/lib/http/connection.rb:141:in `start_tls'

But under 7.2 it works as expected.

Am I doing something wrong or does the new timeout code break Celluloid::IO compatibility?

[zuk]

By the way, I'm not sure how to set httprb to use a different timeout class. If I do the following, I still get the backtrace showing timeout/null.rb:

HTTP
.get("https://www.google.ca/",
  ssl_socket_class: Celluloid::IO::SSLSocket,
  timeout_class: HTTP::Timeout::Global,
  timeout_options: {
    connect_timeout: 5,
    read_timeout: 5,
    write_timeout: 5
  }
)

[ixti]

@zanker seems like we have lost if respond_to? in cycles of refactoring: https://github.com/httprb/http.rb/blob/master/lib/http/timeout/null.rb#L29

[tarcieri]

Yeah, I guess this was in the back of my mind but we never looped back on it.

I think it would be nice if there were a high-level API to wire everything (socket classes, timeouts) up for Celluloid::IO. Something like:

HTTP.backend(:celluloid)

@zuk stick with 0.7 until we sort this out

[ixti]

@zuk Thanks for your report. Will release a patch in a moment.

[ixti]

@tarcieri absolutely in love with HTTP.backend(:celluloid) proposal!

[zuk]

I know this is a bit off topic, but any info on how to switch to a different timeout class (e.g. Global, or PerRequest)? Really having a hard time gleaning this from the recent commits.

[ixti]

@zuk 0.8.2 with fix released. Your variant should work fine (it was failing due to the regression you noticed). So this one should work:

HTTP
.get("https://www.google.ca/",
  ssl_socket_class: Celluloid::IO::SSLSocket,
  timeout_class: HTTP::Timeout::Global,
  timeout_options: {
    connect_timeout: 5,
    read_timeout: 5,
    write_timeout: 5
  }
)

[zuk]

Hate to say it but now getting the following error with 8.2:

.../gems/http-0.8.2/lib/http/timeout/global.rb:26:in `connect_ssl': undefined method `connect_nonblock' for #<Celluloid::IO::SSLSocket:0x007fad44a7e790> (NoMethodError)
    from .../gems/ruby-2.1.5/gems/http-0.8.2/lib/http/timeout/null.rb:31:in `start_tls'

[tarcieri]

@zuk http.rb 0.8+ is going to need a separate timeout backend for Celluloid::IO before it will work correctly. Stick with 0.7 for now.

[ixti]

Hm... I was testing example locally and it was just fine :((

[zanker]

Ah yea, sync_close was missed. Although it looks like Celluloid is leaking SSL sockets since it should set that by default. So I also just enabled that option in Celluloid celluloid/celluloid-io#135

[zanker]

Sorry got a bit busy. Http.rb will work with Celluloid, as long as you don't use timeouts. The null timeout backend should work fine with Celluloid, since the sync_close= fix was made in Http.rb. It'll be fixed in Celluloid::IO whenever they tag a new version.

#216

[ixti]

👍