Skip to content
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

Repaired Fatal Error for Nonexistent URLs #301

Merged
merged 3 commits into from
Aug 9, 2023

Conversation

Windows81
Copy link
Contributor

@Windows81 Windows81 commented Aug 9, 2023

For playlists which contain nonexistent videos, entries may return as None. In that case, the program will print this stack trace:

An exception just occured, if you found this exception isn't related with any of your connection problem, please report this issue to https://github.com/bibanon/tubeup/issues
Traceback (most recent call last):
  File "C:\Users\USERNAME\AppData\Roaming\Python\Python311\site-packages\tubeup\__main__.py", line 99, in main
    for identifier, meta in tu.archive_urls(URLs, metadata,
  File "C:\Users\USERNAME\AppData\Roaming\Python\Python311\site-packages\tubeup\TubeUp.py", line 417, in archive_urls
    downloaded_file_basenames = self.get_resource_basenames(
                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\USERNAME\AppData\Roaming\Python\Python311\site-packages\tubeup\TubeUp.py", line 176, in get_resource_basenames
    if check_if_ia_item_exists(entry) == 0:
       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "C:\Users\USERNAME\AppData\Roaming\Python\Python311\site-packages\tubeup\TubeUp.py", line 111, in check_if_ia_item_exists
    itemname = sanitize_identifier('%s-%s' % (infodict['extractor'],
                                              ~~~~~~~~^^^^^^^^^^^^^
TypeError: 'NoneType' object is not subscriptable

Tested on this playlist, which has one unavailable video.

@mrpapersonic
Copy link
Collaborator

The change at line 184 breaks regular (non-playlist) downloads.

Also, you think you could log a warning to the user instead of just silently not mirroring a video? e.g. [WARN] Video <url> is not available. Skipping.

@vxbinaca
Copy link
Collaborator

vxbinaca commented Aug 9, 2023

For playlists which contain nonexistent videos

It's your job to check URLs are alive. When you rip channels or playlists you do it at your own risk. Use yt-dlp to print out video IDs, pipe into batch file, rip one by one from batch. We can't do everything for you. Trying to do everything for the user increases the devs burden, increases the size of the codebase, and every attempt to do something a user thinks is a good idea leads to code breakage. I'm thinking of comments ripping.

Look into yt-dlps flags.

Not closing this in case you can make it work.

@Windows81
Copy link
Contributor Author

Windows81 commented Aug 9, 2023

Hey. Apologies for the bad commit, and thanks for running the tests! I'll take better care for the unit tests next time. I believe that the two lines I've added in, as insignificant as they are to the codebase, are worth the change for streamlining working on larger playlists.

@Windows81
Copy link
Contributor Author

Windows81 commented Aug 9, 2023

I just released a new commit and made sure that all 20 unit tests came out fine. I removed some redundant code and abstracted that away into a function for your convenience.

@brandongalbraith
Copy link
Collaborator

@Windows81 Haven't had a chance to review yet, but wanted to say thank you for putting the effort into resolving this edge case 👏🏻 🙇‍♂️

@Windows81
Copy link
Contributor Author

You're welcome! It's not really an edge case when you're working with large playlists from channels which have had videos removed (which is almost every one).

@vxbinaca vxbinaca merged commit 2d5d3a3 into bibanon:master Aug 9, 2023
@vxbinaca
Copy link
Collaborator

vxbinaca commented Aug 9, 2023

Thanks @Windows81 new version cut

@Windows81
Copy link
Contributor Author

Hey. I tested it a bit more and made a new pull request for that.

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.

4 participants