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

wicked_pdf and remotipart gems can't coexist #111

Closed
woodwardjd opened this issue May 3, 2012 · 12 comments · Fixed by #925
Closed

wicked_pdf and remotipart gems can't coexist #111

woodwardjd opened this issue May 3, 2012 · 12 comments · Fixed by #925
Labels

Comments

@woodwardjd
Copy link

I don't have the depth of expertise to identify if one or both projects are doing something inadvisable, so I'm posting this on both projects in the hope that someone more knowledgeable will be able to point out a fix to one or both parties. At the very least, these gems don't seem to play well together.

Both gems mess with the render method in ActionController::Base. wicked_pdf does method chaining and remotipart does it by calling super.

When wicked_pdf and remotipart are both loaded in the Gemfile part of remotipart ceases to function.

See JangoSteve/remotipart#27 for more details.

@unixmonkey
Copy link
Collaborator

Yes, it looks like both of our projects muck with render. I need to check the compatibility of this a little bit more, but I think if you just completely remove (or comment-out) the self.included method in pdf_helper.rb, then change your controller code to use render_with_wicked_pdf instead of render:

respond_to do |format|
  format.pdf do
    render_with_wicked_pdf :pdf => 'some_filename'
  end
end

This should let you use them both together until I get a proper fix together. I was never really a fan of the alias_method chaining anyway...

@woodwardjd
Copy link
Author

Played around with it tonight for a bit. Got it working on Rails 3.2.3 with:

  if ::Rails.version < "3.1"
    def self.included(base)
      base.class_eval do
        alias_method_chain :render, :wicked_pdf
        alias_method_chain :render_to_string, :wicked_pdf
        after_filter :clean_temp_files
      end
    end
  else
    def self.included(base)
      base.class_eval do
        after_filter :clean_temp_files
      end
    end
    ActionController::Renderers.add :pdf do |obj, options|
      log_pdf_creation
      options[:basic_auth] = set_basic_auth(options)
      options.delete(:pdf)
      make_pdf((WickedPdf.config || {}).merge(options))
    end
  end

and

...
      html_string = render_to_string(:template => options[:template], :layout => options[:layout], :formats => options[:formats], :handlers => options[:handlers], :prefixes => options[:prefixes])
...

I won't bother committing this exactly because I'm too new to the Rails community to know if this is the best backwards compatible way to do it, and I haven't fully considered if its best to move the disposition/filename stuff to be the responsibility of the calling controller (though I think it is).

@gshen
Copy link

gshen commented Apr 4, 2013

I have forked a branch to use in our project using David's suggestion, I am wondering when can we have the official fix in wicked_pdf in the near future. Thank you for your good work!

@unixmonkey unixmonkey added the bug label Apr 16, 2014
kdiogenes added a commit to kdiogenes/wicked_pdf that referenced this issue Aug 6, 2014
@ilucin
Copy link

ilucin commented Oct 17, 2015

I have the same issue with Turbolinks. Turbolinks is also overriding render and when I add wicked_pdf gem that override stops working. Commenting out self.included (as mentioned above) works but I would like to avoid using forked library just because of this issue.

Since Turbolinks is kinda official rails lib (they are really pushing it) I would try to come up with a solution for this. I'm not a ruby developer so I can't really help much :/

@Grammarella
Copy link

+1

We use the forked branch and have to avoid updating the gem due to this issue.

@unixmonkey
Copy link
Collaborator

This should be solved with the merge of #574

Please reopen if there are any further issues after upgrading.

@pierre-alain-b
Copy link

Hi there,

I see this problem popping up again with wicked_pdf 2.0.1 and remotipart 1.4.x. If I downgrade to wicked_pdf 1.4.0, the problem disappears.

Best
Pierre

@unixmonkey
Copy link
Collaborator

The only thing that I can see that could be causing this for you is this change: https://github.com/mileszs/wicked_pdf/compare/1.4.0..2.0.1#diff-ea9340aeb7bd0bcc5782373373980c56R9

introduced in #869

I think maybe the deferring of loading makes it so that remotipart again get loaded first, causing render issues.

My plan for wicked_pdf version 3 is to stop mucking with render entirely, but it's a big change to the API surface.

Until then, you should be able to do something like this: #111 (comment)

or roll back to the working version. I do plan on working to fix this soonish, but it'll still likely be awhile before it's released unless you, or someone else, can help.

@unixmonkey unixmonkey reopened this Feb 29, 2020
@pierre-alain-b
Copy link

pierre-alain-b commented Feb 29, 2020

First thank you for the prompt reply. I agree the only change that could explain the new behavior would be there:
https://github.com/mileszs/wicked_pdf/pull/869/files#r386040878

I don't know enough the mechanism of ActiveSupport.on_load to understand why it was needed to be compatible with Rails 6 but I agree the deferred loading could explain that remotipart is registered first.

For my culture, if someone could explain what is the interest of the deferred loading, I would appreciate. Performance?

@ryanb
Copy link

ryanb commented Mar 17, 2020

For my culture, if someone could explain what is the interest of the deferred loading, I would appreciate. Performance?

If a reference to ActionController::Base is not deferred it will cause a deprecation warning in Rails 6. See: rails/rails#36546

This is due to an issue with loading classes in an initializer which are managed by the autoloader. Classes referenced in an initializer will be lost when they are reloaded.

ActionText has a hook when ActionController::Base loads it also loads ActionText::ContentHelper and ActionText::TagHelper which are in the autoload path. This means we can't load ActionController::Base until after the app is initialized.

Why are the ActionText classes managed by the autoloader? Internally it is a Rails engine and the classes are defined in app/models which is in the autoload path.

@pierre-alain-b
Copy link

Thanks, I will give it a try when the new version of the gem is out! I look forward to it!

@davegudge
Copy link
Contributor

TL;DR

Update your Gemfile to use the unreleased changes on master to resolve the issue:

# Tracking master to resolve: https://github.com/mileszs/wicked_pdf/issues/891
gem "wicked_pdf", github: "mileszs/wicked_pdf", branch: "master"

I was previously using skillstream@c8c09fa to allow wicked_pdf (v2) and remotipart to coexist.

However to address [the Rails] DEPRECATION WARNING: Initialization autoloaded the constants ..., I upgraded wicked_pdf to the latest version (2.1.0) which introduced the issue below:

rails (~> 6.0.3.7)
wicked_pdf (2.1.0)
remotipart (1.4.4)

fatal (exception reentered):


UncaughtThrowError: uncaught throw :app_exception
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/web-console-4.1.0/lib/web_console/middleware.rb:134:in `throw'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/web-console-4.1.0/lib/web_console/middleware.rb:134:in `rescue in call_app'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/web-console-4.1.0/lib/web_console/middleware.rb:131:in `call_app'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/web-console-4.1.0/lib/web_console/middleware.rb:28:in `block in call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/web-console-4.1.0/lib/web_console/middleware.rb:17:in `catch'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/web-console-4.1.0/lib/web_console/middleware.rb:17:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/show_exceptions.rb:33:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/railties-6.0.3.7/lib/rails/rack/logger.rb:37:in `call_app'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/railties-6.0.3.7/lib/rails/rack/logger.rb:26:in `block in call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.7/lib/active_support/tagged_logging.rb:80:in `block in tagged'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.7/lib/active_support/tagged_logging.rb:28:in `tagged'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.7/lib/active_support/tagged_logging.rb:80:in `tagged'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/railties-6.0.3.7/lib/rails/rack/logger.rb:26:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/sprockets-rails-3.2.2/lib/sprockets/rails/quiet_assets.rb:13:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/remote_ip.rb:81:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/request_store-1.5.0/lib/request_store/middleware.rb:19:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/request_id.rb:27:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/method_override.rb:24:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/runtime.rb:22:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/activesupport-6.0.3.7/lib/active_support/cache/strategy/local_cache_middleware.rb:29:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/executor.rb:14:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/static.rb:126:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/rack-2.2.3/lib/rack/sendfile.rb:110:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/actionpack-6.0.3.7/lib/action_dispatch/middleware/host_authorization.rb:82:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/webpacker-6.0.0.beta.7/lib/webpacker/dev_server_proxy.rb:25:in `perform_request'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/rack-proxy-0.7.0/lib/rack/proxy.rb:63:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/railties-6.0.3.7/lib/rails/engine.rb:527:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/puma-5.3.2/lib/puma/configuration.rb:249:in `call'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/puma-5.3.2/lib/puma/request.rb:77:in `block in handle_request'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/puma-5.3.2/lib/puma/thread_pool.rb:338:in `with_force_shutdown'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/puma-5.3.2/lib/puma/request.rb:76:in `handle_request'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/puma-5.3.2/lib/puma/server.rb:438:in `process_client'
	from /Users/dave/.rbenv/versions/2.6.4/lib/ruby/gems/2.6.0/gems/puma-5.3.2/lib/puma/thread_pool.rb:145:in `block in spawn_thread'

I fixed the issue locally in the gem code located at bundle show wicked_pdf. It was only when I checked out the wicked_pdf gem to submit a PR, that I realised that v2.1.0 != master 🤦.

lib/wicked_pdf/pdf_helper.rb has been modified since the release of 2.1.0, and the changes appear to allow both wicked_pdf and remotipart to coexist.

I've tested the wicked_pdf master branch locally and it appears to work as expected along side remotipart.

Changes to Gemfile to:

# Tracking master to resolve: https://github.com/mileszs/wicked_pdf/issues/891
gem "wicked_pdf", github: "mileszs/wicked_pdf", branch: "master"

Once a new release of wicked_pdf is published, the Gemfile can be reverted to gem "wicked_pdf".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants