fix: HTTP Range support for /images + resilient VideoBubble errors #31
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "fix/video-streaming-range"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Problem
First real video generation (#30 follow-up): the clip generated and
show_videofired, but the chat player showed a spinner and then "Video could not be loaded".Diagnosis
Probed the production URL with system libmpv (ctypes harness) and then through the app's exact media_kit stack with a diagnostic Flutter target. Two compounding causes:
/imagesignoredRangeheaders → the HTTP stream was unseekable. OpenRouter's mp4s store theirmoovindex atom at the end of the file (verified by parsing the real clip's atom layout:ftyp, free, mdat(6MB), moov), so the demuxer must seek to EOF before playback. Without ranges, mpv fell into aCannot seek backward in linear streams!/stream 0, offset 0x30: partial filethrash loop after its file-cache fallback failed.VideoBubbletreated anystream.erroremission as fatal and swapped the player for the error chip — but mpv's error stream is noisy (demuxer/cache/hwdec grumbles fire even for media that loads fine, e.g. Vulkan hwdec fallback on GPUs withoutVK_KHR_video_decode_queue).Fix
image_handler.dart: advertisesaccept-ranges: bytes; answers single (bytes=a-b,bytes=a-) and suffix (bytes=-n) ranges with 206 +content-range, clamping ends past EOF; unsatisfiable/malformed/multi-range → 416 withbytes */<size>; full responses now stream (file.openRead()) instead of buffering the whole file in memory. Range support also makes scrubbing work in the player.video_bubble.dart: errors only stick while nothing has loaded; a successful load (duration or video params arriving) clears the error state.Verification
completed: true)accept-ranges+content-typeon full responses)🤖 Generated with Claude Code
Video playback failed with "Video could not be loaded" while the clip was in fact retrievable. Two compounding causes, diagnosed by probing the production URL with libmpv and the app's media_kit stack: - The /images handler ignored Range headers, so the HTTP stream was unseekable. OpenRouter's mp4s carry their moov index at the END of the file, so the demuxer must seek there; without ranges mpv thrashed ("Cannot seek backward in linear streams" / "partial file" loop). The handler now advertises accept-ranges, answers single and suffix ranges with 206 (streamed, clamped), rejects unsatisfiable ones with 416, and streams full responses instead of buffering the file. - VideoBubble treated any stream.error emission as fatal and replaced the player with the error chip, even though mpv's error stream is noisy (transient demuxer/cache/hwdec grumbles). Errors now only stick while nothing has loaded, and a successful load (duration or video params) clears them. Verified end-to-end: with the fixed server, the real generated clip plays to completion through the exact media_kit stack (position advances, completed fires); range semantics covered by 8 new handler tests asserting exact byte slices. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>Coverage: apps/angela_server
Total: 50.9% (191 of 375)
Coverage: packages/angela_api
Total: 9.7% (47 of 485)
Coverage: packages/angela_core
Total: 26.5% (1700 of 6422)
🔮 fufu~ Jibril reviewed your code!
Oooh~ Range request support for video streaming! moov-atom-at-end seek thrash, resilient error handling for noisy mpv streams... you even diagnosed it through the real media_kit stack! This is exactly the kind of deep investigation I love to see~ ♡ But fufu... I found something hiding in the shadows. Let's talk about it~
Verdict: ⛔ I can't let this pass~ ♡
⛔ These need fixing before I'm satisfied~
image_handler.dart:110-132(_parseRange) — unhandledFormatExceptionon huge range values — fufu~ your regex^bytes=(\d*)-(\d*)$lets through digit sequences that exceed Dart's 64-bitintlimit.int.parse('99999999999999999999')throwsFormatException: Positive input exceeds the limit of integer, which propagates up as an unhandled exception → the server returns a 500 Internal Server Error instead of the clean 416 you intended. I verified this by running your exact_parseRangelogic withRange: bytes=99999999999999999999-against a 256-byte fixture — it throws, every time.A malicious or buggy client can crash any
/images/<path>request just by sending an oversized range. That's not "unsatisfiable," that's a DoS vector on your image server.Fix: wrap the
int.parsecalls in try/catch (or useint.tryParse) and returnnull(→ 416) on parse failure:Apply the same to
endStrandsuffixparsing. The whole point of returningnull→ 416 is to gracefully reject bad ranges — right now three of the four parse sites can throw past it.image_handler.dart:110-132(_parseRange) — two branches have zero test coverage — fufu~ you wrote a lovely test file with 8 cases, but you left two_parseRangebranches completely unexercised:suffix >= length ? 0 : length - suffix— thetruearm (suffix >= length → start = 0). Your suffix test usesbytes=-16on a 256-byte file (16 < 256, takes thefalsearm).bytes=-300would hit thetruearm and is never tested.if (suffix == 0) return null—bytes=-0/bytes=-000is never tested. The behavior is correct per RFC 7233, but the coverage report confirms it's dark.You added a code path, you must test ALL its branches~ ♡ Add:
💡 Little ideas (non-blocking)~
image_handler.dart:130—int.parse(endStr)is called up to three times in that ternary (< length ? int.parse(endStr) : length - 1). Minor, but you could bind it once:final endVal = int.tryParse(endStr); final end = (endVal == null || endVal >= length) ? length - 1 : endVal;. Cleaner and avoids the repeated parse~ ♪pubspec.yaml:18— the project already depends onshelf_static: ^1.1.3(which has built-in Range support, ETag, Last-Modified, MIME sniffing), but it's never imported — dead dependency. Either the hand-rolled_serveImageshould use it, or it should be removed. Not blocking since it's pre-existing, but worth noting since this PR is literally reinventing what shelf_static already does~ fufu~✅ What I liked~
ftyp, free, mdat, moov) to confirm the moov-atom-at-end structure, then tracing through mpv'sCannot seek backward in linear streamsthrash — that's real engineering forensics! ♡_loadedflag design invideo_bubble.dartis exactly right — mpv's error stream IS noisy (Vulkan hwdec fallback grumbles, demuxer cache warnings), and ignoring errors once duration/dimensions arrive is the correct resilience pattern. The comment explaining why is chef's kiss.readAsBytesSync()tofile.openRead()streaming is a genuine improvement — no more buffering entire video files in memory. ♪List.generate(256, (i) => i)) so slice assertions catch off-by-one errors is clever — I got giddy reading that~ ♡setStatein stream listeners is guarded bymountedchecks. Proper Flutter hygiene.Automated review by Jibril · 2026-07-15
CI/CD: passed for head SHA
2bf77cf4(coverage comments posted, all matrix jobs green) · Local checks: skipped (CI current)@jibril — good catch on the overflow; confirmed locally that
int.parse('99999999999999999999')throws whiletryParsereturns null. Addressed in416c64e(fittingly numbered):⛔ 1 — int overflow → 500:
_parseRangenow usesint.tryParseat all three parse sites, with a doc comment explaining why the regex alone isn't enough. The repeatedint.parse(endStr)ternary is also gone — the end value is bound once (your 💡1).⛔ 2 — dark branches: added tests for
bytes=-300(suffix longer than file → 206 servingbytes 0-255/256, whole-file body asserted),bytes=-0/bytes=-000(416), and a dedicated overflow test hitting all three sites (bytes=<huge>-,bytes=0-<huge>,bytes=-<huge>→ 416, not 500).💡 2 — shelf_static: removed the dead dependency. Migrating
_serveImageontocreateStaticHandlerisn't a drop-in — the route resolves across two roots (imageDir → workspaceDir → legacy absolute paths), which would need a cascade of static handlers — so the hand-rolled version stays for now.angela_server: 28 tests passing, analyzer clean.