ostinelli / net-http2

NetHttp2 is an HTTP/2 client for Ruby.
MIT License
141 stars 31 forks source link

Sidekiq crashes when 'Connection reset by peer' occurs #15

Closed alexbuijs closed 7 years ago

alexbuijs commented 7 years ago

Hi, I am using this gem to send push notifications through APNS with a JWT token. Because I am using the JWT token and not the certificate authentication, I am using this gem directly instead of apnotic. Today was the first day we took this approach with the latest version (0.15.0) to production. After several hundreds of sent notifications, Sidekiq unfortunately crashes:

2017-03-13T15:46:21.032Z 25384 TID-gnuy3xkr4 PushJob JID-b5fa698e2653cdfede8fe080 INFO: start
Connection reset by peer
/opt/rbenv/versions/2.3.0/lib/ruby/2.3.0/openssl/buffering.rb:178:in `sysread_nonblock'
/opt/rbenv/versions/2.3.0/lib/ruby/2.3.0/openssl/buffering.rb:178:in `read_nonblock'
/home/rails/backend/shared/bundle/ruby/2.3.0/gems/net-http2-0.15.0/lib/net-http2/client.rb:133:in `block in socket_loop'
/home/rails/backend/shared/bundle/ruby/2.3.0/gems/net-http2-0.15.0/lib/net-http2/client.rb:130:in `loop'
/home/rails/backend/shared/bundle/ruby/2.3.0/gems/net-http2-0.15.0/lib/net-http2/client.rb:130:in `socket_loop'
/home/rails/backend/shared/bundle/ruby/2.3.0/gems/net-http2-0.15.0/lib/net-http2/client.rb:102:in `block (2 levels) in ensure_ope

I have read several issues like this (e.g. https://github.com/ostinelli/apnotic/issues/17), but not with using this gem directly. Am I doing something wrong? I am using the synced approach like this:

def ios_notification(token)
  client = NetHttp2::Client.new(apns_config.url)
  response = client.call(:post, apns_config.path + token, body: ios_payload, headers: ios_headers)
  raise JSON.parse(response.body)['reason'] unless response.ok?
rescue => exception
  Appsignal.set_error(PushError.new(exception, token, 'ios'))
ensure
  client.close
end

Thanks for any pointers to solve this issue!

ostinelli commented 7 years ago

From the README:

Thread-Safety [...] Errors in the underlying socket loop thread will be raised in the main thread at unexpected execution times, unless you specify the :error callback on the Client (recommended).

and the example provided there is:

client.on(:error) { |exception| puts "Exception has been raised: #{exception}" }

Does this cover your use case?

By the way, there's an open PR (https://github.com/ostinelli/apnotic/pull/28) to add support for token auth, maybe you'd like to contribute?

ostinelli commented 7 years ago

BTW there is an issue discussing this, here it is if you'd like to take a look: https://github.com/ostinelli/net-http2/issues/4

alexbuijs commented 7 years ago

Thanks, I'll test it out and will look at the PR!

alexbuijs commented 7 years ago

I cannot reproduce the 'Connection reset' error manually. I suspect the problem is that I open and close the connection to APNS many times in a short time frame. Apple is probably denying requests: APNs treats rapid connection and disconnection as a denial-of-service attack.. As your apnotic gem already has a solution that deals with sending in pools from sidekiq I will try to use that with JWT.

alexbuijs commented 7 years ago

Adding the error callback prevented sidekiq from crashing and reusing the client helped with the connection reset issue. Thanks!