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

Raise exceptions control #212

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

avpeshkov
Copy link

Hi all, I've been working with pdfkit and found several cases where wkhtmltopdf outputs errors, but still generates a pdf. I would like to be able to manually control how the pdf-kit responds to errors.

ps: I apologize if I violate protocol in any way, this is my first time doing pull request to opensource.

@JazzCore
Copy link
Owner

Hi, thanks for the work! Not sure about it though, since when using this option any errors will be ignored, even when pdf is not generated. More correct way is to check if we got any output and if there are errors - we can ignore those, and if output is missing - properly raise an exception

@avpeshkov
Copy link
Author

Hi, thanks for the work! Not sure about it though, since when using this option any errors will be ignored, even when pdf is not generated. More correct way is to check if we got any output and if there are errors - we can ignore those, and if output is missing - properly raise an exception

Hey,
Thanks for the quick review! I have taken your remarks into account and updated pull request, also I added info about the changes to README.

@avpeshkov
Copy link
Author

Hi, again:)
I fixed tests to get python2 compatibility. Who would have thought there would be such a mistake in workflow:)

@JazzCore
Copy link
Owner

JazzCore commented Dec 3, 2021

Looks like GH CI is broken atm... No time to review at the moment, sorry. I'll try to test it on this weekend

@avpeshkov
Copy link
Author

Thank you, this errors looks vary unusual.
No problem, we all have a job:)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants