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

Do not unlink HTML temp files immediately. #950

Merged

Conversation

joshuapinter
Copy link
Contributor

The reason for not wanting to unlink them is primariily for debugging. If an error occurs, the wkhtmltopdf command that had problems is shown as part of the error message. Something like this:

Failed to execute:
["/Users/me/.rbenv/versions/2.6.5/bin/wkhtmltopdf", "--enable-local-file-access", "--disable-smart-shrinking", "--orientation", "Portrait", "--page-size", "Letter", "--margin-top", "27", "--margin-bottom", "19", "--header-html", "file:////var/folders/9n/vjrv4bzs7fxf6crkdc84tf740000gn/T/wicked_header_pdf20201023-55115-le3quo.html", "--footer-html", "file:////var/folders/9n/vjrv4bzs7fxf6crkdc84tf740000gn/T/wicked_footer_pdf20201023-55115-1o98lhe.html", "file:////var/folders/9n/vjrv4bzs7fxf6crkdc84tf740000gn/T/wicked_pdf20201023-55115-1am8ara.html", "/var/folders/9n/vjrv4bzs7fxf6crkdc84tf740000gn/T/wicked_pdf_generated_file20201023-55115-qwkucc.pdf"]
Error: Invalid argument @ io_fread - ### /var/folders/9n/vjrv4bzs7fxf6crkdc84tf740000gn/T/wicked_pdf_generated_file20201023-55115-qwkucc.pdf

However, the references to the HTML temp files used in that command are no longer there so it's very difficult to reproduce or investigate further.

We want to keep these files around so that you can essentially run the nearly identical wkhtmltopdf command when there was an error and dig deeper into it.

Keeping these files does not keep them forever. They are, afterall, temp files and will be cleaned up automatically by the system.

To accomplish this, we use .close instead of .close!, which does not unlink the file immediately after closing it.

NOTE: We still want to use .close! on the generated PDF files for two reasons:

  1. This generated PDF file is not important when an error has occurred and you are re-running the wkhtmltopdf command.

  2. The generated PDF file is the largest of the files. Typically, the HTML files for header, footer and body are relatively small compared to the generated PDF file.

Fixes #948.

The reason for not wanting to unlink them is primariily for debugging. If an error occurs, the `wkhtmltopdf` command that had problems is shown as part of the error message. However, the references to the HTML temp files used in that command are no longer there so it's very difficult to reproduce or investigate further.

We want to keep these files around so that you can essentially run the nearly identical `wkhtmltopdf` command when there was an error and dig deeper into it.

Keeping these files does not keep them forever. They are, afterall, temp files and will be cleaned up automatically by the system.

To accomplish this, we use `.close` instead of `.close!`, which does not unlink the file immediately after closing it.

NOTE: We still want to use `.close!` on the generated PDF files for two reasons:

1. This generated PDF file is not important when an error has occurred and you are re-running the `wkhtmltopdf` command.

2. The generated PDF file is the largest of the files. Typically, the HTML files for header, footer and body are relatively small compared to the generated PDF file.

Fixes mileszs#948.
@unixmonkey unixmonkey merged commit 39311ef into mileszs:master Oct 27, 2020
@unixmonkey
Copy link
Collaborator

Thanks!

@joshuapinter joshuapinter deleted the do_not_unlink_html_temp_files_immediately branch October 27, 2020 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible to keep the html around instead of deleting it?
2 participants