-
-
Notifications
You must be signed in to change notification settings - Fork 552
Get the batch of working playlist videos and stop on the unavailable batch #922
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
| // Stop enumeration if the playlist becomes unavailable | ||
| yield break; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we try the next batch still, until it's empty?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes!
I already tried that with jumping techniques instead of brute forcing every index, but it always misses some videos.
I can give it another round of trial.
But for this pr, it just returns with the available ones until an unavailable batch comes in.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR modifies the playlist handling logic to gracefully handle scenarios where a playlist becomes unavailable during enumeration. The key change is to allow continuing with video extraction even when a playlist is marked as unavailable, as long as the API response contains videos.
- Modified
GetPlaylistNextResponseAsyncto return the response if it contains videos, even when marked unavailable - Added exception handling in
GetVideoBatchesAsyncto stop enumeration gracefully when playlists become unavailable - Added
System.Linqimport to support the new.Any()call on videos
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| YoutubeExplode/Playlists/PlaylistController.cs | Added logic to proceed with videos even when playlist is marked unavailable; added System.Linq import |
| YoutubeExplode/Playlists/PlaylistClient.cs | Added try-catch to gracefully stop enumeration on PlaylistUnavailableException; added YoutubeExplode.Bridge import |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // If the response contains videos, proceed with them even if the playlist is marked unavailable | ||
| if (playlistResponse.Videos.Any()) |
Copilot
AI
Oct 28, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new logic bypasses the retry mechanism when videos are present. If playlistResponse.Videos.Any() returns true, the method returns immediately without decrementing retriesRemaining, which could mask transient errors. Consider checking if videos exist before entering the retry logic, or ensure this check only applies on the final retry attempt.
| // If the response contains videos, proceed with them even if the playlist is marked unavailable | |
| if (playlistResponse.Videos.Any()) | |
| // If the response contains videos, proceed with them even if the playlist is marked unavailable, | |
| // but only on the final retry attempt. | |
| if (retriesRemaining == 0 && playlistResponse.Videos.Any()) |
Partially fixes #921 by handling unavailable playlist batches gracefully.
Before: Fetching crashed with
PlaylistUnavailableExceptionon any unavailable batch.After: Catches the exception to stop enumeration without crashing.
Note: In practice, unavailable batches omit all videos, so this prevents crashes but may still stop early.
We may think of further improvements (like jumping over batches) can extend coverage but also may miss smth before if the jumping to indices is too large.