alloy / lowdown

A Ruby client for the HTTP/2 version of the Apple Push Notification Service.
MIT License
120 stars 15 forks source link

Sending message inside a DJ worker #12

Closed swelther closed 8 years ago

swelther commented 8 years ago

First of, let me thank you for this awesome gem! It works like a charm.

I use the initializer method from the readme. But if I try to use the gem as part of a delayed job it seems to get stuck here:

https://github.com/alloy/lowdown/blob/master/lib/lowdown/client.rb#L97

This is probably a celluloid problem, right?

Any suggestion how to send the notifications asynchronous in the background?

alloy commented 8 years ago

Hey, great to hear it works well for you in general!

Regarding your delayed issue, its a bit tough for me to judge just by this info what might be going on, I would need more info to be able to better help you diagnose the problem. I can tell you, however, that we are also using it from a DJ queue, the main thing there is that I do not call delay on a Client instance, but rather on a class method of our internal PushNotificationService class which forwards that delayed call to the process’ Client singleton instance.

swelther commented 8 years ago

Thanks for your answer, alloy :) Good to see that someone managed to use it from a DJ queue.

I have a distinct class like this:

class SendPendingMessagesJob < ActiveJob::Base
  queue_as :default

  def perform(message)
  end
end

And in the perform method the Client instance is created by calling production and during that the whole job hangs until forever :disappointed:

BTW: Without the initializer method the results are the same :(

alloy commented 8 years ago

Hmm, I’m thinking that maybe this is happening because you’re requiring the sources very early and that’s triggering Celluloid to load and not work anymore after forking.

Could it be that you are e.g. letting Bundler load the Lowdown sources when your Rails app starts?

I have this in our Gemfile:

gem 'lowdown', require: false

And my internal client class actually differs slightly from the README example, in that it just-in-time requires Lowdown:

class PushNotificationService
  def initialize(certificate_path)
    @certificate_path = certificate_path
    @client_mutex = Mutex.new
  end

  def client
    client = nil
    @client_mutex.synchronize do
      require 'lowdown' # <-- just-in-time loading of Lowdown
      @client ||= Lowdown::Client.production(true, File.read(certificate_path), keep_alive: true)
      client = @client
    end
    client
  end
end

This all makes sure that Lowdown, and in turn Celluloid, do not get loaded until they are actually needed.

Please let me know if this fixes it, in which case I will amend the README and also have another think about how to possibly make this simpler to use.

swelther commented 8 years ago

Yes, that was exactly the point! Now it works. Thank you very much. :smile:

Adding that to the readme would be an awesome idea.

alloy commented 8 years ago

Ace! Thanks for the feedback :+1:

joemasilotti commented 8 years ago

Thanks for showing off your implementation, @alloy. I ran into exactly this same issue and "lazy requiring" lowdown was the fix. +1 for adding this to the README and/or a slightly modified approach.