fix(download): stream episode downloads to disk to avoid iOS OOM (#113)#218
Merged
Conversation
Episode downloads buffered the entire file in memory (requestUrl -> response.arrayBuffer -> vault.createBinary), which OOM-kills Obsidian on memory-constrained iOS WebViews for large episodes (issue #113). Stream the download instead: fetch in 4 MiB HTTP Range chunks and append each straight to disk (adapter.writeBinary + appendBinary), so peak heap stays at roughly one chunk regardless of episode size. Collapse concurrent downloads of the same episode into a single in-flight transfer, surface progress in the notice, and fall back to the legacy whole-file path when the adapter lacks binary append or the server ignores Range.
Addresses thermo-nuclear review of the #113 streaming download: - delete the half-written file and validate total bytes on any mid-stream failure, so a truncated partial can't later be served as a complete download (the legacy createBinary path was atomic; appendBinary is not). - extract a single resolveDownloadTarget() so the extension/playability/path pipeline lives in one place instead of being duplicated per branch. - dedupe concurrent downloads by episode identity (podcast+title) rather than stream URL; collapse the duplicated success Notice; centralize the adapter cast; drop downloadFile's now-unused onFinished/onError options. - add unit coverage for the streaming path: 206 multi-chunk append, 200 whole-body fallback, and mid-stream-failure cleanup.
… cached files Thermo-review follow-ups (all behaviour-preserving; verified on-device): - Split the 1k-line downloadEpisode.ts into focused modules: - download/streaming.ts — Range probe + chunked appendBinary writer - download/mediaSignatures.ts — pure magic-byte / ISO-BMFF detectors downloadEpisode.ts drops from 1070 to 837 lines. Co-located unit tests added for both; the streaming tests drive the chunk loop with a small chunk size (206 multi-chunk, 200 whole-body, unknown-total EOF, mid-stream error). - L1: skip the multi-MB Range probe when the episode is already on disk under its URL extension. The "Download Playing Episode" command is ungated and previously re-probed on every invocation (~1.2s of wasted network for an already-downloaded episode; now ~20ms with no request). - L2: make chunkSize a real, defaulted parameter instead of threading the module constant through every call site, and exercise it in unit tests. - Replace a stray raw NUL byte in the dedupe key with an escaped separator.
Deploying podnotes with
|
| Latest commit: |
7d58faa
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://c828bb3f.podnotes.pages.dev |
| Branch Preview URL: | https://fix-ios-download-streaming-1.podnotes.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8e38bae4ea
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
…218) Addresses Codex review feedback: a 206 partial response's Content-Length is the chunk size, not the file total. The previous fallback adopted it as the total, so writeStreamedFile stopped after the first chunk and recorded a truncated download as success. Now Content-Length is only used for the 200 whole-body case; a 206 with an unknown total (bytes 0-N/*) keeps totalSize null and relies on the short-chunk EOF heuristic. Adds a regression test.
…exed (#218) Addresses review feedback: streamed downloads are written through the vault adapter, so a freshly written partial may not be in the vault index yet and deleteEpisodeFile's getAbstractFileByPath could miss it, leaving bytes behind on a failed download. Fall back to adapter.exists/adapter.remove so cleanup is index-independent. Updates the mid-stream-failure test to assert the adapter-removal path.
This was referenced Jun 23, 2026
|
🎉 This PR is included in version 2.17.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Downloading an episode crashes Obsidian on iOS (#113). The download path buffered the entire episode in memory —
requestUrl()→response.arrayBuffer→vault.createBinary— with no streaming. On iOS's tight per-process WebView memory budget, a large episode (long audio, audiobooks, video) exceeds the limit and the OS kills the app. Desktop has the headroom, so it only reproduces on mobile.Fix
Stream the download instead of buffering it:
download/streaming.ts): probe the first 4 MiB with aRangeheader, then pull the rest in 4 MiB chunks, appending each straight to disk viaadapter.writeBinary(first chunk) +adapter.appendBinary(rest). Peak heap is ~one chunk regardless of episode size.requestUrlis kept (it bypasses mobile CORS); a server that ignoresRange(answers200) falls back to a single whole-file write, and an adapter without binary append falls back to the legacycreateBinarypath — so nothing is ever truncated.createBinarypath was atomic; chunked append is not).Verification
Reproduced and fixed on a physical iPhone (iOS 26.5.1) via the WebKit Web Inspector:
vite buildclean.Notes
src/downloadEpisode.tssplit into focused modules (download/streaming.ts,download/mediaSignatures.ts) with co-located unit tests; the file drops from 1070 to 837 lines.npm run typecheckfailures are confined totests/e2e/(missingobsidian-e2emodule) and are unrelated to this change.Closes #113