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

Fix #2537 - Build fails on bad image file #2637

Closed
wants to merge 1 commit into from

Conversation

@adarshdec23
Copy link
Contributor

adarshdec23 commented Jan 14, 2017

Catch OSError and return.
Log the exception as a warning. The warning message is consistent with that in Pillow docs:

IOError – If the file cannot be found, or the image cannot be opened and identified.

@Kwpolska
Copy link
Member

Kwpolska commented Jan 14, 2017

👎

If you have a broken file, Nikola crashing is the correct behavior, because that file probably won’t be recognized by browsers either, and you will end up with garbage output. In my opinion, this can be left as-is for the user to fix themselves.

@adarshdec23
Copy link
Contributor Author

adarshdec23 commented Jan 14, 2017

I see the value of what you're saying.
Bad images should not reach the browser. So isn't it better to log an error and not process the image further. This ensures that the image does not reach the output and also users won't have to see an ugly back-trace.

@Kwpolska
Copy link
Member

Kwpolska commented Jan 14, 2017

A hard crash works better here IMO than just a warning and continuing with a build.

@Kwpolska Kwpolska closed this Jan 14, 2017
@felixfontein
Copy link
Contributor

felixfontein commented Jan 14, 2017

How about improving the error message by printing it with self.logger.error() and then calling sys.exit()? Then the user won't see the ugly stack trace, and the build will still stop.

@adarshdec23 adarshdec23 deleted the adarshdec23:f2537 branch Jan 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.