Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Allow missing assets #188

Closed
wants to merge 4 commits into from

3 participants

Dmytrii Nagirniak Ian Cordasco Marcus Vorwaller
Dmytrii Nagirniak

This PR removes the inconsistency between Linux and OSX versions of wkhtmltopdf.

When there are missing assets (images, JS etc):

  • on Linux (x64): wkhtmltopdf exits with status code = 2 and does produce actual PDF (but an error is raised)
  • on OSX: wkhtmltopdf exits with status code = 0 and does produce actual PDF (and no error is raised)

This also means that no failures will occur during local development on OSX, but it is more likely to fail in production.

The spec here may need a bit of adjustment depending on what platform it is being run on, but should be a good starting point.

(This also incorporates the #187)

Ian Cordasco

Could you address the test failures (preferably by not modifying the tests themselves) on Travis?

Ian Cordasco sigmavirus24 referenced this pull request
Open

Exitstatus #187

Dmytrii Nagirniak

There are a few problems with the tests that I would prefer someone else to look at. A lot of the tests are failing on Travis even prior to this PR (but passing locally on OSX) so I was concentrating on the only 2 in PR.

The original description isn't accurate now after I updated to the recent wkhtmltopdf-binary gem.
Now the given spec fails with status code=1 on OSX but succeeds on Linux (status code = 0).

So it looks like there are also some differences between versions of the wkhtmltopdf as well as platform.

Not sure how to deal with it now? Maybe just using the ignore-load-errors flag should do the job.
Looks like it would be PITA to deal with all those version/platform issues as part of pdfkit.

Opinions?

(BTW, the #187 can be merged separately)

Ian Cordasco

@dnagir I think we're finally on our way towards having Travis successfully run tests. Could you just rebase this branch against master so we can see if the tests pass on Travis? Thanks!

Ian Cordasco

@dnagir do you have the chance to rebase this PR?

SeaLink sealink referenced this pull request
Merged

Allow missing assets #209

Ian Cordasco

Closed since #209 ostensibly handles the same issue.

Ian Cordasco sigmavirus24 reopened this
Ian Cordasco

Actually looking at the diffs, #209 and this are different enough

Marcus Vorwaller

I'm seeing the opposite behavior, with PDFKit 0.6.1 on OSX with missing images it fails with:

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

If I revert to PDFkit 5.x it generates the pdf with broken images as expected.

Ian Cordasco
Owner

@dnagir could you rebase this pull request?

Dmytrii Nagirniak

@sigmavirus24 I'm not sure if I should do this. I'm out of touch with this PR now and don't remember particulars already. People also reported the opposite behaviour to this PR, such as #209.
So maybe it would be better to look there?

Ian Cordasco

Ok @dnagir. Sorry this sat so long without a good response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 8, 2013
  1. Dmytrii Nagirniak
  2. Dmytrii Nagirniak

    Unfocus

    dnagir authored
Commits on Aug 9, 2013
  1. Dmytrii Nagirniak
Commits on Aug 15, 2013
  1. Dmytrii Nagirniak

    Fix spelling

    dnagir authored
This page is out of date. Refresh to see the latest.
Showing with 21 additions and 3 deletions.
  1. +14 −2 lib/pdfkit/pdfkit.rb
  2. +7 −1 spec/pdfkit_spec.rb
16 lib/pdfkit/pdfkit.rb
View
@@ -67,10 +67,10 @@ def to_pdf(path=nil)
pdf.close_write
pdf.gets(nil)
end
+ status = $? # $? is thread safe per http://stackoverflow.com/questions/2164887/thread-safe-external-process-in-ruby-plus-checking-exitstatus
result = File.read(path) if path
- # $? is thread safe per http://stackoverflow.com/questions/2164887/thread-safe-external-process-in-ruby-plus-checking-exitstatus
- raise "command failed: #{invoke}" if result.to_s.strip.empty? or !$?.success?
+ raise "command failed (exitstatus=#{status.exitstatus}): #{invoke}" unless successful?(result, status)
return result
end
@@ -140,4 +140,16 @@ def normalize_value(value)
end
end
+ def successful?(result, status_info)
+ result.to_s.strip.empty? or successful_exit_statuses.include?(status_info.exitstatus)
+ end
+
+ def successful_exit_statuses
+ # Some of the codes: https://code.google.com/p/wkhtmltopdf/issues/detail?id=1088
+ [
+ 0, # all good
+ 2 # returned when assets are missing (404): https://code.google.com/p/wkhtmltopdf/issues/detail?id=548
+ ]
+ end
+
end
8 spec/pdfkit_spec.rb
View
@@ -205,7 +205,13 @@
it "should throw an error if it is unable to connect" do
pdfkit = PDFKit.new("http://google.com/this-should-not-be-found/404.html")
- lambda { pdfkit.to_pdf }.should raise_error
+ lambda { pdfkit.to_pdf }.should raise_error /exitstatus=1/
+ end
+
+ it "should generate PDF if there are missing assets" do
+ pdfkit = PDFKit.new("<html><body><img alt='' src='http://example.com/surely-it-doesnt-exist.gif' /></body></html>")
+ pdf = pdfkit.to_pdf
+ pdf[0...4].should == "%PDF" # PDF Signature at the beginning
end
end
Something went wrong with that request. Please try again.