Skip to content

fix: keep downloaded videos when SponsorBlock is unavailable - #966

Merged
dialmaster merged 2 commits into
DialmasterOrg:devfrom
gauvainw:fix/sponsorblock-timeout
Oct 26, 2026
Merged

dialmaster merged 2 commits into
DialmasterOrg:devfrom
gauvainw:fix/sponsorblock-timeout

Conversation

@gauvainw

@gauvainw gauvainw commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #965.

A SponsorBlock lookup timeout can currently mark a successfully downloaded video as failed and remove it from the download archive, so the next run downloads it again.

Treat the unprefixed SponsorBlock API error as a warning, keep successful videos in the archive, and report the number of affected videos in the download's live completion message. This count is not persisted in job notes or included in run summaries or notifications. Repeated error lines for the same video count once. The Preprocessing: form and other video download failures still count as failures.

SponsorBlock API 403 errors and their preceding WARNING: retry lines no longer trigger YouTube cookie advice or cause an unrelated video's failure to be auto-retried as a 403. Genuine video download 403s are still detected.

Tests cover stdout and stderr, exit codes 0 and 1, the three retry warnings emitted with --extractor-retries 3, archive retention, notification success/failure totals, and mixed failure cases. The troubleshooting guide describes the resulting behavior and where the SponsorBlock count appears.

Testing

  • The review regression checks reproduce eight failures before the fix and pass afterward; all 159 tests in the two targeted suites pass.
  • Backend: 6,239 tests passed across 212 suites; line coverage 91.29%.
  • Frontend: 5,497 tests passed across 297 suites; line coverage 89.77%.
  • ESLint, TypeScript checks, and the client production build passed.
  • Documentation checks: 18 tests passed; the documentation production build passed.
  • All four npm audit commands used by CI passed at their configured thresholds (runtime: high; all dependencies: critical).
  • Rebased on current dev (71fcf937), which includes the dependency fixes from fix(deps): patch body-parser and multer security issues #967. This PR does not change dependency manifests or lockfiles relative to dev.

The regression tests use simulated process output. No live YouTube download or SponsorBlock outage was exercised. Docker/Compose integration was verified for the original revision; it was not rerun for this review revision because this workstation has no container runtime.

@dialmaster dialmaster left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for picking this up!

I only see a few small issues -- see my review comments :)

Comment thread docs/TROUBLESHOOTING.md Resolved
Recognize WARNING lines before the final SponsorBlock ERROR, reproduce all
three extractor retries in the executor tests, and cover an unrelated
video failure that must not inherit a false HTTP 403 auto-retry.

Clarify that the SponsorBlock count appears in the live completion message.
@gauvainw
gauvainw force-pushed the fix/sponsorblock-timeout branch from 97c7127 to 51cf95e Compare October 26, 2026 16:38
@gauvainw

Copy link
Copy Markdown
Contributor Author

Thank for the review ! I love the project man that's really great

@dialmaster
dialmaster merged commit 3e62fa8 into DialmasterOrg:dev Oct 26, 2026
13 checks passed
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.

2 participants