fix: continue with live archive processing if ffmpeg task fails

[Zibbp]

Fixes #1059

[coderabbitai]

Walkthrough

Adds file-existence and non-empty validations for video post-processing and move steps, updates live-download worker finalization behavior, extends task-name mapping for live variants, and adds tests for file validation plus a new (duplicated) live ffmpeg-kill integration test that polls queue task status and kills ffmpeg during tests.

Changes

Cohort / File(s)	Summary
Live download worker & tests internal/tasks/live_video.go, internal/live/live_test.go	Changed DownloadLiveVideoWorker to not abort on non-cancellation download errors and to record a local downloadStatus on finalization. Expanded chat-download lookup to include Available/Pending/Scheduled/Running/Retryable. Added test helper waitForQueueTaskStatus, an os/exec alias import, and a new TestTwitchWatchedChannelLiveFFmpegKilledStillFinalizes test (duplicated in-file) that kills ffmpeg and asserts downstream task progression.
Video processing validations internal/tasks/video.go	Added validateNonEmptyFile(path string, label string) error, imported os, and added pre-flight checks in PostProcessVideoWorker.Work and MoveVideoWorker.Work to verify input files/playlists exist and are non-empty before processing or moving (including HLS/live variants).
Unit tests for validation internal/tasks/video_test.go	New unit tests TestValidateNonEmptyFile covering missing file, empty file, and non-empty file cases using temp directories and os.WriteFile.
Task name mapping internal/utils/enum.go	Extended GetTaskName to map TaskDownloadLiveVideo→TaskDownloadVideo and TaskDownloadLiveChat→TaskDownloadChat.

🚥 Pre-merge checks | ✅ 3 | ❌ 3

❌ Failed checks (3 warnings)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	⚠️ Warning	The addition of file validation tests and helper functions in video.go/video_test.go appear tangentially related to robustness, but are not directly required by #1059's scope of handling ffmpeg/HTTP 5XX failures.	Clarify whether file validation changes are necessary for the ffmpeg error handling fix, or split them into a separate pull request to keep scope focused.
Docstring Coverage	⚠️ Warning	Docstring coverage is 55.56% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Merge Conflict Detection	⚠️ Warning	❌ Merge conflicts detected (7 files): ⚔️ go.mod (content) ⚔️ go.sum (content) ⚔️ internal/live/live_test.go (content) ⚔️ internal/tasks/live_video.go (content) ⚔️ internal/tasks/video.go (content) ⚔️ internal/transport/http/vod.go (content) ⚔️ internal/utils/enum.go (content) These conflicts must be resolved before merging into main.	Resolve conflicts locally and push changes to this branch.

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and concisely describes the main change: enabling live archive processing to continue even when ffmpeg task fails, which directly addresses the linked issue #1059.
Description check	✅ Passed	The description links to the related issue #1059, providing context about the problem being fixed, though minimal detail about the solution.
Linked Issues check	✅ Passed	The changes address the core requirement of #1059 by allowing live video downloads to continue finalization after ffmpeg failures, preventing the queue from hanging in in-progress state.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix-live-download-crash

⚔️ Resolve merge conflicts (beta)

• Auto-commit resolved conflicts to branch fix-live-download-crash
• Create stacked PR with resolved conflicts
• Post resolved changes as copyable diffs in a comment

---

No actionable comments were generated in the recent review. 🎉

🧹 Recent nitpick comments

internal/tasks/video.go (1)

132-146: Minor: redundant os.Stat call — utils.FileExists already stats the file.

utils.FileExists calls os.Stat internally, then line 137 calls os.Stat again. You can consolidate into a single stat:

♻️ Suggested simplification

 func validateNonEmptyFile(path string, label string) error {
-	if !utils.FileExists(path) {
-		return fmt.Errorf("missing %s: %s", label, path)
-	}
-
 	info, err := os.Stat(path)
 	if err != nil {
-		return fmt.Errorf("failed to stat %s %s: %w", label, path, err)
+		return fmt.Errorf("missing or inaccessible %s %s: %w", label, path, err)
 	}
 	if info.Size() == 0 {
 		return fmt.Errorf("empty %s: %s", label, path)
 	}
-
 	return nil
 }

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

internal/tasks/live_video.go (1)

140-150: ⚠️ Potential issue | 🟡 Minor

Download status marked Success even on ffmpeg failure—downstream tasks lack pre-checks for missing/empty files.

The design allows downstream tasks to run (and fail with errors if the video is missing or invalid), but there are no explicit file existence or size validation checks before PostProcessVideo or MoveVideo attempt operations. While the system doesn't silently create corrupt archives (both tasks will fail and error out), the resilience relies entirely on implicit error propagation from underlying system calls rather than explicit guards. Consider adding pre-flight validation (e.g., FileExists checks) in these workers to fail fast and more gracefully when the download produces no output.

🧹 Nitpick comments (3)

internal/tasks/live_video.go (2)

70-92: Pre-existing goroutine leak: chat-download goroutine can never exit after ctx reassignment.

The goroutine at line 72 captures the ctx variable by reference. After ctx is reassigned to context.Background() (line 102 or 105), ctx.Done() inside the goroutine becomes a nil channel (never fires). If startChatDownload receives no further values, the goroutine blocks forever.

This existed before for the cancel path and is now extended to the error path. Consider closing the startChatDownload channel after the download call returns, or capturing the original context in a separate variable for the goroutine.

Proposed fix: close channel to unblock goroutine

 	// download live video
+	// Close startChatDownload after download completes to unblock the goroutine.
+	defer close(startChatDownload)
 	downloadErr := exec.DownloadTwitchLiveVideo(ctx, dbItems.Video, dbItems.Channel, startChatDownload)

Note: this requires updating the goroutine to handle the channel close (a receive on a closed channel returns the zero value immediately). Alternatively, restructure the goroutine to use a dedicated done channel.

Also applies to: 102-102, 105-105

---

124-133: Finalization errors after context replacement cause the entire worker to fail — consider whether this is desired.

If getTaskId (line 119) or JobCancel (line 129) fails after ctx has been replaced with context.Background(), the worker returns an error, but MaxAttempts is 1 (line 29), so there are no retries. This means a transient DB error during finalization could leave the archive stuck despite having a valid video file. This is pre-existing behavior for the cancel path, but now applies more broadly.

Consider wrapping these finalization calls with best-effort error handling (log + continue) rather than hard-failing, similar to how downloadErr itself is now treated.

internal/live/live_test.go (1)

109-127: Helper name is misleading — it only checks TaskVideoDownload.

waitForQueueTaskStatus sounds generic, but line 121 hardcodes the check to q.TaskVideoDownload. If this helper is reused for other task fields, it will silently check the wrong one.

Consider renaming to waitForQueueVideoDownloadStatus or parameterizing the field check.