A proposed fix to wkhtmltopdf as it sometimes fails even generation is mostly successful

[noshbar]

Hi, thanks for your wrapper, it's really great and easy to use.

As the title states, sometimes the wkhtmltopdf binary completes, prints "Done", then prints an error message stating

Exit with code 2 due to http error: 404 Page not found

As a result the wrapper fails to complete and the file is never moved from /tmp to the final destination, despite the file (probably) being okay.

I'm sure this is not a problem with your wrapper, and I'm pretty sure it's something I've done wrong, but it seems to appear to other people:
https://code.google.com/p/wkhtmltopdf/issues/detail?id=548

So I have a proposed "fix" (read: hack) to work around it for now.
I'm new to Git and GitHub, so I've made a patch but I don't know how to post it.

Below are the contents (please keep laughing to a minimum):

diff --git a/WkHtmlToPdf.php b/WkHtmlToPdf.php
index d2c6279..27f43de 100644
--- a/WkHtmlToPdf.php
+++ b/WkHtmlToPdf.php
@@ -307,7 +307,17 @@ class WkHtmlToPdf
             $result = proc_close($process);
 
             if($result!==0)
-                $this->error = "Could not run command $command:\n$stderr";
+            {
+                if ( (!file_exists($fileName)) || (filesize($fileName)===0) )
+                {
+                    $this->error = "Could not run command $command:\n$stderr";
+                }
+                else
+                {
+                    $this->error = 'Warning: an error occurred while creating the PDF, but some data was written';
+                    return true;
+                }
+            }
         } else
             $this->error = "Could not run command $command";

[mikehaertl]

Seems to make sense. I've added your changes. Can you try the latest version from master?

BTW you can find out more on how to contribute to a project here: https://help.github.com/articles/fork-a-repo

[mikehaertl]

@noshbar Have you tested the latest master?

[noshbar]

@mikehaertl Hi. Sorry for the delay in response.
I have not tested it yet, but having looked at your change I don't think it satisfies my need.

I'm looking for the function to return true even though the wkhtmltopdf process returns a non-zero exit code, which your change doesn't do.

That said, I think me posting my original was a mistake as there really is an issue with the binary not your wrapper. What I have done is a hack, and the change you have committed is great.

So for me, I will keep my line where I return true if a file exists, but I think you can safely close this issue for now, and I'm really sorry for wasting your time!

Thanks again.

[mikehaertl]

Ok, thanks.