Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

please deprecate this gem for rails 5.x #26

Open
ngouy opened this issue Jun 7, 2017 · 10 comments
Open

please deprecate this gem for rails 5.x #26

ngouy opened this issue Jun 7, 2017 · 10 comments

Comments

@ngouy
Copy link

ngouy commented Jun 7, 2017

It's no more usable with rails 5.x

@Bilkhaan
Copy link

Yes i am facing issue with using this gem with actionmailer 5.1.0 version. The initialize method dont accept any parameters in actionmailer's base.rb. And somehow sidekiq_mailer is sending number of parameters in initialize call which cause an error.

Did you find any solution or alternative?

@ngouy
Copy link
Author

ngouy commented Aug 30, 2017

I think it's naturally handled now with r5 mailers and sidekiq. We do not use anything else to send async mails. Just send your email from a sidekiq worker

@Bajena
Copy link

Bajena commented Mar 19, 2019

Hey, I found a solution when updating Rails from 4.2 -> 5.0.1. It's a bit hacky, but does the job if you still wanna use sidekiq-mailer.

I monkey patched it in an initializer:

require "sidekiq_mailer"

module Sidekiq
  module Mailer
    # Original implementation of Sidekiq::Mailer used method_missing to prepare
    # a proxy object, then initialize a mailer instance and use it to send the
    # actual message.
    # Since Rails 5 it's not possible to send a message by initializing
    # a mailer - it needs to be done by calling the class method.
    #
    # Let's add this special arg to method calls to inform the mailer class
    # that we want to generate the actual message.
    SPECIAL_ARG = :from_sidekiq_proxy

    class Proxy
      # Original method:
      # https://github.com/andersondias/sidekiq_mailer/blob/master/lib/sidekiq_mailer/proxy.rb#L10
      def actual_message
        @actual_message ||= @mailer_class.send(
          @method_name, *(@args + [SPECIAL_ARG])
        ).message
      end
    end

    module ClassMethods
      # Original method:
      # https://github.com/andersondias/sidekiq_mailer/blob/master/lib/sidekiq_mailer.rb#L61
      def method_missing(method_name, *args)
        if args.include?(SPECIAL_ARG) ||
          !action_methods.include?(method_name.to_s)
          super(method_name, *(args - [SPECIAL_ARG]))
        else
          Sidekiq::Mailer::Proxy.new(self, method_name, *args)
        end
      end
    end
  end
end

it works fine for us, but I'm not sure if it's not too "hacky" in order to post it as a PR. What do you think guys?

@ngouy
Copy link
Author

ngouy commented Mar 19, 2019

actually the better patch is to get rid of this gem. Rails 5 and sidekiq are perfectly fine to send mails async without it

@Bajena
Copy link

Bajena commented Mar 19, 2019

Thanks for response @ngouy! Do you know an easy way to replace this gem in Rails 5 without changing a lot of codebase?

@ngouy
Copy link
Author

ngouy commented Mar 19, 2019

it was almost one year ago x)

can you make a small pastebin to see how your mailer looks like and how do you call it ?

@ngouy
Copy link
Author

ngouy commented Mar 19, 2019

take a look at ActionMailer::Base
The idea is to call a mailer into a worker

Ultimately you can create a mailer_worker which takes your mailer argument plus the method (mail) you want to send

Such as MyMailerWorker.perform_async(AccountMailer, "welcome", user.id)

And your Mailer worker just do
something like

def perform(mailer_class, mailer_method, *params)
    mailer_class.constantize.send(mailer_method, *params)
end

and this way your mailer should remain the same I guess

@Bajena
Copy link

Bajena commented Mar 19, 2019

It's nothing special:

class ApplicationMailer < ActionMailer::Base
  ##
  # The asynchronous delivery is throttled at Sidekiq::Mailer::Worker#perform
  # directly as we want to throttle attempts at delivery by the worker, not the
  # initial call to mailer, which only pushes the task to the queue.
  #
  # See config/initializers/sidekiq_mailer.rb for the actual rate-limiting
  # code and quotas.
  include ApplicationMailerHelper
  include Sidekiq::Mailer
  include LoggedDelivery
  include DefaultSender
  include AwsSesConfiguration
end

we're also patching sidekiq mailer with another patch that throttless the sendout:

RATE_LIMIT = 28

module Sidekiq
  module Mailer
    module RateLimiter
      def perform(*)
        limiter.within_limit { super }
      end

    private

      def limiter
        @limiter ||= Sidekiq::Limiter.window(
          "aws-ses",
          RATE_LIMIT, :second, wait_timeout: 10
        )
      end
    end
  end
end

Sidekiq::Mailer::Worker.prepend(Sidekiq::Mailer::RateLimiter)

@ngouy
Copy link
Author

ngouy commented Mar 19, 2019

maybe the good way to do it is to keep your monkey patch if it's working and let a # TODO for later. First migrate your version, then refactor. The less you do in one operation, the less you do mistakes

@Bajena
Copy link

Bajena commented Mar 19, 2019

Ok, thanks, I'll let you know here if I ever change it 💃

@ngouy ngouy closed this as completed Aug 19, 2019
@ngouy ngouy reopened this Aug 19, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants