Pass on error code in checkProcessStatus #328
Conversation
|
@leimd Do you have a link to an issue on wkhtmltopdf repo with more details about this issue? Ideally, this sort of error should not leak out of snappy. Instead of providing the user a way to ignore this error, we should ignore it when we know it can be. |
|
Hi @NiR- , It looks like there are several issues open on wkhtmltopdf that relates to what we are experiencing. Exit with code 1 due to network error: ContentNotFoundError unreachable IMG src path, with invalid ext on URL gets ContentNotFoundError My goal of this PR is really to pass on the error code to the end user. This way the end user can decide what to do with the error code. For our use case, we're not so strict so that error code 1 is acceptable, but other users whose more strict can re throw the Exception if they want. (default behavior) |
|
@leimd Ok fine :) I'm gonna merge it, but first could you add an entry in the FAQ please? |
|
@NiR- Thanks. Looks like there's some conflict, I will resolve the conflict and add an entry to the FAQ. Once I finish these I will ping you again. |
leimd
force-pushed
the
pass-on-status-code-in-checkProcessStatus
branch
3 times, most recently
from
95b4e10
to
8da4a4c
Compare
|
Hi @NiR- , I've resolved the conflict and updated the FAQ. However styleci seems to be failing for unknown reason and appveyor is failing as well. Would you mind to take a look at those? sorry I don't have much experience with the aforementioned two services. -Tom |
|
|
||
| $this->expectException(\RuntimeException::class); | ||
| $r->invokeArgs($media, [1, '', 'Could not connect to X', 'the command']); | ||
|
|
There was a problem hiding this comment.
StyleCI complains about whitespaces on this line. Could you leave it empty please?
|
Don't worry about the error on AppVeyor: it's a sporadic error happening for whatever reason. I'll try to rebuild your PR a few time to get it green. |
leimd
force-pushed
the
pass-on-status-code-in-checkProcessStatus
branch
from
5605249
to
07d85f5
Compare
|
@NiR- Good news! Comment addressed, styleci passing now. looks like it's ready to merge |
|
Thanks @leimd :) AppVeyor jobs are still randomly failing but whatever... |
There is a known issue in wkhtmltopdf where it will generate the pdf and still return error code 1. People have opened issues in the past for this problem: #257 #90 .
By adding the error code to the exception thrown by checkProcessStatus, it gives the end user the ability to check for the error code and catch this exception and swallow it if possible.