Skip to content

Fix media silently not saving to Photos (Save to Camera Roll)#2186

Open
estevecastells wants to merge 1 commit into
TelegramMessenger:masterfrom
estevecastells:fix/save-to-camera-roll-silent-failures
Open

Fix media silently not saving to Photos (Save to Camera Roll)#2186
estevecastells wants to merge 1 commit into
TelegramMessenger:masterfrom
estevecastells:fix/save-to-camera-roll-silent-failures

Conversation

@estevecastells

Copy link
Copy Markdown

Summary

Saving media to the iOS Photo Library can silently fail — the save appears to complete (progress HUD dismisses as if successful) but the photo/video never lands in Photos. This has been reproducible for a long time and is very annoying in daily use (high impact: it's the core "Save to Camera Roll" action).

The root cause is that the authorized save closure in submodules/SaveToCameraRoll/Sources/SaveToCameraRoll.swift swallows every failure on the way to PHPhotoLibrary:

  • Infinite hang, nothing saved (motion/Live photos): a failed AVAssetExportSession hits guard success else { return } and never calls putCompletion(), so the signal hangs forever and nothing is written.
  • Crash: addAssetIdentifierToJPEG(imageData, ...)! force-unwraps and traps on any image the re-encode can't handle.
  • False success: the completion handler always emits 1.0 + completion even when the change block staged zero resources or PhotoKit returned an error. PhotoKit reports an empty performChanges as success, so a failed save is indistinguishable from a real one.
  • Wrong container extension: every video is copied to a hardcoded …/<random>.mp4 temp path, discarding the source's real extension (e.g. .mov).

This is still present on master (ffd82647); the recent engine refactor didn't touch this logic.

Changes

  • Preserve the source file's real extension for the temp copy instead of forcing .mp4.
  • Replace the ! force-unwrap with a guard let.
  • On export failure or a failed paired (motion-photo) write, fall back to saving the plain still image so it still lands, and always finish the signal so the HUD dismisses.
  • Track whether a resource was actually staged and log when media did not land, instead of reporting unconditional success.

Public signature is unchanged; all call sites are unaffected.

Proof

The full app build is Bazel/device-only, so I verified the control-flow fix with a standalone Swift harness that transcribes the OLD and NEW authorized-closure flow over a real FileManager with PhotoKit/export results injected:

Scenario                                  | result
----------------------------------------------------------------------------------------------------
Motion photo, video export FAILS          | FIXED ✅
    OLD: completed=NO   assetLanded=NO   crashed=no
    NEW: completed=YES  assetLanded=YES  crashed=no
Motion photo, JPEG re-encode returns nil  | FIXED ✅
    OLD: completed=NO   assetLanded=NO   crashed=YES
    NEW: completed=YES  assetLanded=YES  crashed=no
Motion photo, paired save FAILS           | FIXED ✅
    OLD: completed=YES  assetLanded=NO
    NEW: completed=YES  assetLanded=YES
Plain .mov video (extension handling)     | OLD videoExt=mp4 (forced) → NEW videoExt=mov (preserved)

i.e. the old flow either hangs, crashes, or reports success while saving nothing; the new flow always completes and the media lands.

🤖 Generated with Claude Code

When saving a photo/video from a chat to the iOS photo library, the save
could intermittently "glitch" and never appear in Photos. The authorized
save closure swallowed every failure on the way to PHPhotoLibrary:

- A failed AVAssetExportSession in the motion-photo path hit
  `guard success else { return }` and never called putCompletion(), so the
  signal hung forever and nothing was saved.
- `addAssetIdentifierToJPEG(...)!` force-unwrapped, crashing on any image
  the re-encode could not handle.
- The completion handler always reported success even when the change
  block staged nothing or PhotoKit returned an error, so a failed save was
  indistinguishable from a successful one.
- Videos were always copied to a hardcoded ".mp4" temp path, discarding the
  source container's real extension.

Make the path honest and resilient:

- Preserve the source file's extension for the temp copy.
- Replace the force-unwrap with a guard.
- On export failure or a failed paired (motion-photo) write, fall back to
  saving the plain still image so it still lands, and always finish the
  signal so the progress HUD dismisses.
- Track whether a resource was actually staged and log when media did not
  land, instead of reporting an unconditional success.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.

1 participant