-
Notifications
You must be signed in to change notification settings - Fork 648
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
Fix header/footer temp file is removed before wkhtmltopdf called #1039
Conversation
…tmltopdf is called
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed bug in production environment as listed here: #1031 (comment)
Edit @ 2023-01-08 - forked version working fine since the original comment date
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great! Sorry it took a bit for me to take a closer look.
I did post a few questions if you don't mind answering.
stderr.read | ||
end | ||
_out, err, status = Open3.capture3(*command) | ||
err = [status.to_s, err].join("\n") if !err.empty? || !status.success? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the change from popen3
to capture3
here, and the joining?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@unixmonkey I just reuse this PR Use Open3.capture3 to avoid deadlock between stdout/stderr in subprocess
@@ -79,6 +79,7 @@ def pdf_from_url(url, options = {}) | |||
rescue StandardError => e | |||
raise "Failed to execute:\n#{command}\nError: #{e}" | |||
ensure | |||
clean_temp_files |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -61,7 +61,7 @@ def parse_header_footer(options) | |||
r += make_options(opt_hf, [:line], hf.to_s, :boolean) | |||
if options[hf] && options[hf][:content] | |||
@hf_tempfiles = [] unless defined?(@hf_tempfiles) | |||
@hf_tempfiles.push(tf = WickedPdf::Tempfile.new("wicked_#{hf}_pdf.html")) | |||
@hf_tempfiles.push(tf = File.new(Dir::Tmpname.create(["wicked_#{hf}_pdf", '.html']) {}, 'w')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason WickedPdf::Tempfile
doesn't work here? I remember adding that because an earlier version of Ruby didn't allow tempfiles to be created with an extension, which I know is no longer the case, but don't remember when that changed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@unixmonkey WickedPdf::Tempfile
still can created the temp file. But the files is deleted before the pdf file generated.
This is now in version 2.7.0 of wicked_pdf. Thank you for your help! |
Fix issue #1031.
Header/footer generated by
WickedPdf::Tempfile
is deleted before wkhtmltopdf called.This PR try to use normal
File
then delete it when the pdf file generated.Temporary fix in your Gemfile.
gem 'wicked_pdf', git: 'https://github.com/thanhcuong1990/wicked_pdf.git', branch: 'master'