mileszs / wicked_pdf

PDF generator (from HTML) plugin for Ruby on Rails
http://www.mileszs.com/wicked-pdf-plugin
MIT License
3.52k stars 640 forks source link

fix: check if file exists before deleting #1074

Open LuanGB opened 10 months ago

LuanGB commented 10 months ago

fixes #1073

unixmonkey commented 10 months ago

FYI, this issue also may exist here: https://github.com/mileszs/wicked_pdf/blob/master/lib/wicked_pdf.rb#L46-L50 where the files are closed without checking they exist first.

I'm curious how this is a new problem after the upgrade though... It seems that if any other process clearing tempfiles (a deploy maybe?) could cause this to happen in both old and new versions.

It might also be nice to delete or preserve the header & footer tempfiles with that same :delete_temporary_files option and call close or close! based on it.

LuanGB commented 10 months ago

I'm suspecting that this may be related to this PR: https://github.com/mileszs/wicked_pdf/pull/1039 Cannot put my finger on what exactly is causing it, however

unixmonkey commented 10 months ago

BTW, although it is very tiny, there could be a race-condition where the file exists while checking if the file exists and deleting it. Maybe it'd be better if closing a tempfile were rescue-able than checking for existence. Should speed things up too.

LuanGB commented 10 months ago

That's a good idea. I'll put something together soon.

unixmonkey commented 9 months ago

Ping.

alex-kovshovik commented 3 months ago

I ran into the same issue, probably will have to use the forked version until this fix is released.