-
-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
better handling of preview generation errors #4620
Conversation
@icewind1991, thanks for your PR! By analyzing the history of the files in this pull request, we identified @butonic, @rullzer and @tanghus to be potential reviewers. |
Makes sense but I could not verify that it failed before. I took a document and renamed it to .png and triggered the occ preview:generate-all - before and after there is no error. |
You need an actual corrupted image, not just a wrongly named one |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense!
Do you have one? |
Codecov Report
@@ Coverage Diff @@
## master #4620 +/- ##
=============================================
- Coverage 54.28% 31.34% -22.94%
- Complexity 22086 22090 +4
=============================================
Files 1360 1360
Lines 84659 84063 -596
Branches 1325 1325
=============================================
- Hits 45958 26352 -19606
- Misses 38701 57711 +19010
|
I also added some error handling for the other endpoints that uses that method. |
lib/private/Preview/Generator.php
Outdated
@@ -325,6 +327,10 @@ private function generatePreview(ISimpleFolder $previewFolder, ISimpleFile $maxP | |||
$preview->resize(max($width, $height)); | |||
} | |||
|
|||
if (!$preview->valid()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be moved up? Like right after the getImage call?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
Signed-off-by: Robin Appelman <[email protected]>
Signed-off-by: Morris Jobke <[email protected]>
Signed-off-by: Morris Jobke <[email protected]>
Signed-off-by: Robin Appelman <[email protected]>
bfb8588
to
2847e9f
Compare
Prevents failing hard when working with corrupted images