[PR #162] [MERGED] [downloader] Fix failure diagnostics: exit codes, tracebacks, timeout messages #163

New issue

Closed

opened 2026-10-13 19:48:12 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-13 19:48:12 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/Glitchy-Sheep/boosty-downloader/pull/162
Author: @Glitchy-Sheep
Created: 8/19/2026
Status: ✅ Merged
Merged: 8/19/2026
Merged by: @Glitchy-Sheep

Base: main ← Head: fix/failure-diagnostics

📝 Commits (3)

3f27ca3 fix: Make --post-url failures visible in the exit code
4a17180 fix: Translate every yt-dlp failure into an application error
11d2006 fix: Diagnose a mid-download network timeout as a timeout

📊 Changes

9 files changed (+301 additions, -45 deletions)

View changed files

📝 CHANGELOG.md (+6 -0)
📝 boosty_downloader/src/application/use_cases/download_single_post.py (+7 -4)
📝 boosty_downloader/src/application/use_cases/download_specific_post.py (+11 -24)
📝 boosty_downloader/src/cli/commands/download.py (+7 -3)
📝 boosty_downloader/src/infrastructure/external_videos_downloader/external_videos_downloader.py (+5 -7)
📝 boosty_downloader/src/infrastructure/file_downloader.py (+6 -2)
➕ test/unit/file_downloader/timeout_diagnosis_test.py (+106 -0)
📝 test/unit/use_cases/download_specific_post_test.py (+56 -5)
➕ test/unit/use_cases/ext_video_errors_test.py (+97 -0)

📄 Description

[downloader] Fix failure diagnostics: exit codes, tracebacks, timeout messages

Summary

Three user-facing bugs found by the architecture research sweep, fixed in one PR - all three are about the same theme: when something fails, the app must say so honestly.

Problems and fixes

1. --post-url reported every failure as success. boosty-downloader download -p <url> && echo ok printed ok on a dead link, a missing post, a paywalled post - and even on Ctrl+C. The use case computed an outcome and threw it away. Now the outcome reaches the CLI: failures exit with code 1, Ctrl+C propagates to the top-level handler and exits 130, same as bulk runs. The private duplicated outcome enum is gone - both paths share PostOutcome.

2. A removed or restricted external video crashed the run with a raw traceback. The yt-dlp wrapper raised the bare ExtVideoError base class, but the error translation caught only its subclasses - the most common failure (video deleted, geo-blocked) flew past the whole error model. The raise site now uses the specific ExtVideoDownloadError, the translation catches the base as a safety net for future kinds, and the base class carries video_url so the failed log always names the video.

3. A network timeout mid-download was reported as "Failed during I/O operation". The timeout branch was dead code - it caught an exception nothing raises - so a sock_read timeout fell into the disk-failure diagnosis and sent users checking their drive instead of their network. Real timeouts (including aiohttp's SocketTimeoutError, which inherits from both the timeout and connection families) now map to "Download timed out: the server stopped sending data", placed before the connection branch so the right family wins.

Verification

task ci green: checks, 174 unit tests + 1 strict xfail (10 new tests across the three fixes), e2e, build
New tests pin each bug's story: exit-0-on-failure, the bare base class escaping translation (4 variants), all three timeout classes plus a control that connection errors still map correctly
Changelog updated (3 entries under Fixed)

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/Glitchy-Sheep/boosty-downloader/pull/162 **Author:** [@Glitchy-Sheep](https://github.com/Glitchy-Sheep) **Created:** 8/19/2026 **Status:** ✅ Merged **Merged:** 8/19/2026 **Merged by:** [@Glitchy-Sheep](https://github.com/Glitchy-Sheep) **Base:** `main` ← **Head:** `fix/failure-diagnostics` --- ### 📝 Commits (3) - [`3f27ca3`](https://github.com/Glitchy-Sheep/boosty-downloader/commit/3f27ca3b11b413d226777deeca5edae64d98a872) fix: Make --post-url failures visible in the exit code - [`4a17180`](https://github.com/Glitchy-Sheep/boosty-downloader/commit/4a1718085d5b66facf07d3731b0806504ba7653f) fix: Translate every yt-dlp failure into an application error - [`11d2006`](https://github.com/Glitchy-Sheep/boosty-downloader/commit/11d20062d7d4a7e5e81f24b4316e184862995992) fix: Diagnose a mid-download network timeout as a timeout ### 📊 Changes **9 files changed** (+301 additions, -45 deletions) <details> <summary>View changed files</summary> 📝 `CHANGELOG.md` (+6 -0) 📝 `boosty_downloader/src/application/use_cases/download_single_post.py` (+7 -4) 📝 `boosty_downloader/src/application/use_cases/download_specific_post.py` (+11 -24) 📝 `boosty_downloader/src/cli/commands/download.py` (+7 -3) 📝 `boosty_downloader/src/infrastructure/external_videos_downloader/external_videos_downloader.py` (+5 -7) 📝 `boosty_downloader/src/infrastructure/file_downloader.py` (+6 -2) ➕ `test/unit/file_downloader/timeout_diagnosis_test.py` (+106 -0) 📝 `test/unit/use_cases/download_specific_post_test.py` (+56 -5) ➕ `test/unit/use_cases/ext_video_errors_test.py` (+97 -0) </details> ### 📄 Description # [downloader] Fix failure diagnostics: exit codes, tracebacks, timeout messages ## Summary Three user-facing bugs found by the architecture research sweep, fixed in one PR - all three are about the same theme: when something fails, the app must say so honestly. ## Problems and fixes **1. `--post-url` reported every failure as success.** `boosty-downloader download -p <url> && echo ok` printed `ok` on a dead link, a missing post, a paywalled post - and even on Ctrl+C. The use case computed an outcome and threw it away. Now the outcome reaches the CLI: failures exit with code 1, Ctrl+C propagates to the top-level handler and exits 130, same as bulk runs. The private duplicated outcome enum is gone - both paths share `PostOutcome`. **2. A removed or restricted external video crashed the run with a raw traceback.** The yt-dlp wrapper raised the bare `ExtVideoError` base class, but the error translation caught only its subclasses - the most common failure (video deleted, geo-blocked) flew past the whole error model. The raise site now uses the specific `ExtVideoDownloadError`, the translation catches the base as a safety net for future kinds, and the base class carries `video_url` so the failed log always names the video. **3. A network timeout mid-download was reported as "Failed during I/O operation".** The timeout branch was dead code - it caught an exception nothing raises - so a `sock_read` timeout fell into the disk-failure diagnosis and sent users checking their drive instead of their network. Real timeouts (including aiohttp's `SocketTimeoutError`, which inherits from both the timeout and connection families) now map to "Download timed out: the server stopped sending data", placed before the connection branch so the right family wins. ## Verification - `task ci` green: checks, **174 unit tests + 1 strict xfail** (10 new tests across the three fixes), e2e, build - New tests pin each bug's story: exit-0-on-failure, the bare base class escaping translation (4 variants), all three timeout classes plus a control that connection errors still map correctly - Changelog updated (3 entries under Fixed) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-13 19:48:12 +00:00