Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Gracefully handle failing to thumbnail images #16211

Merged
merged 3 commits into from
Aug 30, 2023
Merged

Gracefully handle failing to thumbnail images #16211

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0329ed4
Gracefully handle failing to thumbnail images
erikjohnston Aug 30, 2023
c9d811b
Newsfile
erikjohnston Aug 30, 2023
c868fa1
Update changelog.d/16211.bugfix
erikjohnston Aug 30, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/16211.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix bug where uploading images would fail if we could not generate thumbanils for them.
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
5 changes: 5 additions & 0 deletions synapse/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,14 @@
import sys
from typing import Any, Dict

from PIL import ImageFile

from synapse.util.rust import check_rust_lib_up_to_date
from synapse.util.stringutils import strtobool

# Allow truncated JPEG images to be thumbnailed.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's a truncated JPEG? E.g. we only have the first say 60% of the JPEG file?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, which turns out to be relatively common for reasons beyond understanding.

ImageFile.LOAD_TRUNCATED_IMAGES = True

# Check that we're not running on an unsupported Python version.
#
# Note that we use an (unneeded) variable here so that pyupgrade doesn't nuke the
Expand Down
5 changes: 4 additions & 1 deletion synapse/media/media_repository.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,10 @@ async def create_content(
user_id=auth_user,
)

await self._generate_thumbnails(None, media_id, media_id, media_type)
try:
await self._generate_thumbnails(None, media_id, media_id, media_type)
except Exception as e:
logger.info("Failed to generate thumbnails: %s", e)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a warning? It is tough though since it is user content, what's an admin to do?

I think catching the error and having the image uploaded even if thumbnails fail is the correct thing to do though!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW my model of warning is "something went wrong, but it wasn't our fault". I can see this one going either way; no strong opinions from me.


return MXCUri(self.server_name, media_id)

Expand Down