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
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing bug where uploading images would fail if we could not generate thumbnails for them.
5 changes: 5 additions & 0 deletions synapse/__init__.py
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
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
Loading