fix(download): stream to a temp file then move into place#220
Merged
Conversation
Appending each range chunk straight to the final vault path made the growing, half-written media file visible to Obsidian's file watcher: a single download fired ~12 create/modify events on the .mp3. Watcher plugins (Waypoint, Dataview, Obsidian Git, MOC/index generators) re-scan the file on every event, and on a memory-constrained mobile WebView that re-scan storm - racing the chunked writer - OOM-crashes the app. Reported against Waypoint, but the cause is the event storm, not any one plugin. Stream every chunk to a dot-prefixed sibling temp that Obsidian keeps out of the file index (zero watcher events while it grows), then rename the completed, size-verified file into place - exactly one create of the finished file, the same shape the pre-#113 atomic createBinary path had. - prefer adapter.rename (in-place, buffers nothing, preserving the #113 memory win); fall back to copy+remove; never re-buffer the whole file, which would reintroduce the OOM this path exists to avoid - unique-per-attempt temp names + an active-set-guarded folder sweep, so colliding or concurrent downloads can't corrupt each other's temps and partials orphaned by a killed download get reclaimed - size verification still runs before the move; on failure the temp is removed, never the final path Validated on Android (Obsidian 1.12.7 + Waypoint 3.0.1): the matched scenario that OOM-crashed now completes, modify events on the final file drop from ~12 to 0 (one create), byte-perfect across megaphone, simplecast and wnyc, heap bounded, no orphan temp left behind.
Deploying podnotes with
|
| Latest commit: |
948ab77
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://97b27ac1.podnotes.pages.dev |
| Branch Preview URL: | https://fix-download-temp-then-move.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: af6ebe773c
ℹ️ 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".
| const dir = slash === -1 ? "" : filePath.slice(0, slash + 1); | ||
| const name = slash === -1 ? filePath : filePath.slice(slash + 1); | ||
| const token = `${Date.now().toString(36)}-${(partialCounter++).toString(36)}`; | ||
| return `${dir}.${name}.${token}${PARTIAL_SUFFIX}`; |
There was a problem hiding this comment.
For streaming-capable adapters, a title/template that safeDownloadFilePath already caps to the filesystem's 255-unit filename limit now gets expanded again here by adding the leading dot, token, and .podnotes-partial suffix before writeBinary runs. Those long episodes previously wrote to the capped final path, but now the temp component exceeds the same per-file limit and the download fails with an overlong filename; truncate the temp basename or reserve space for the suffix when constructing it.
Useful? React with 👍 / 👎.
partialPathFor prepended a dot and appended ".<token>.podnotes-partial" to the already-length-capped final name, which could push the temp's name segment past the 255-unit ENAMETOOLONG limit for a maxed-out title - so a very long episode that downloaded fine before could fail on the new temp-then-move write path. Put the unique token first (so it survives the tail truncation) and run the temp path through enforceMaxPathLength.
rename/copy/remove/list are public, required members of Obsidian's DataAdapter - only appendBinary is runtime-only - so the optional guards and the copy+remove fallback in moveIntoPlace guarded a state that cannot occur (anything past the writeBinary+appendBinary gate is a full DataAdapter with rename). Extend the real DataAdapter for the single appendBinary bolt-on, correct the now-accurate comment, finalize with a plain adapter.rename, and drop the unreachable fallback - which also removes a latent "copy succeeded but remove threw, so a complete download is reported failed" bug. Collapse the streaming branch's three catch->delete->rethrow blocks into one finally guard.
github-actions Bot
pushed a commit
that referenced
this pull request
Jun 25, 2026
## [2.17.2](2.17.1...2.17.2) (2026-06-25) ### Bug Fixes * **download:** stream to a temp file then move into place ([#220](#220)) ([5aee276](5aee276))
|
🎉 This PR is included in version 2.17.2 🎉 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.
The remaining crash
#113 fixed the download downloader's own peak heap (whole-file buffer → bounded 4 MiB Range chunks, released in 2.17.1). But the crash was still reported on mobile, and on a matched Android setup (Obsidian 1.12.7 + Waypoint 3.0.1) it reproduces deterministically.
Root cause: the streaming download appended each chunk straight to the final vault path. That makes the growing, half-written
.mp3visible to Obsidian's file watcher - a single 47 MB download fires 1 create + ~11 modify events on the media file (measured). Watcher plugins (Waypoint here, but equally Dataview, Obsidian Git, MOC/index generators) re-read/re-scan the file on every event, and ~11 re-reads of a growing media file on a memory-constrained mobile WebView - racing the chunked writer - OOM-kills the renderer. The trigger is the event storm, not any one plugin.The fix: stream to a temp, then move into place
Stream every chunk to a dot-prefixed sibling temp (
.{name}.{token}.podnotes-partial) that Obsidian keeps out of the file index entirely - so it fires zero watcher events while it grows - thenadapter.renamethe completed, size-verified file into place. Watchers then see exactly one create of an already-complete file - the same shape the pre-#113 atomiccreateBinarypath produced.moveIntoPlace: preferadapter.rename(in-place, buffers nothing → preserves BUG|Crash on Android phone when download podcast #113's memory win), fall back tocopy+remove, otherwise throw. It deliberately does not fall back toreadBinary→writeBinary: re-buffering the whole 50-200 MB file would reintroduce the BUG|Crash on Android phone when download podcast #113 OOM (and can trip the open AndroidwriteBinarylarge-file hang) on the exact devices this targets.filePath(Transcription transcribing other episode but not the one playing. #107/Default download.path is empty, breaking the Download command (writes ".mp3" to vault root, 2nd download fails) #183), so a per-path temp + a blind sweep would let concurrent downloads corrupt each other. Unique names + an in-flightSetguard make that impossible, and reclaim partials orphaned by a killed download.Validation (Android emulator, Obsidian 1.12.7 + Waypoint 3.0.1)
.mp3Triangulated across 3 CDN families (megaphone, simplecast, wnyc) with Waypoint on - all complete, 0 modify events, no orphan temp, the moved file is index-resolvable (playback works), heap stays bounded. (Note: podtrac/simplecast ad-stitch per-UA, so total size varies per request; byte-perfection is verified by PodNotes' internal
total === probe.totalSizegate, not an external size compare.)Compatibility
createBinary) branch untouched - it was already a single atomic write.Tests
771 unit tests pass; lint + typecheck clean. New coverage:
partialPathFor/isPartialPath,moveIntoPlace(rename / copy+remove / throw-no-buffer),sweepStalePartials(orphan vs active vs real file; best-effort), and end-to-end streaming asserting the dotfile temp, the single rename, no orphan, index-resolvability, move-failure cleanup, and the stale-partial sweep.Fixes the crash reported on #113.