JuanBindez / pytubefix

Python3 library for downloading YouTube Videos.
http://pytubefix.rtfd.io/
MIT License
653 stars 91 forks source link

Debug Information Included with Exception Message #267

Closed jhanley-com closed 3 weeks ago

jhanley-com commented 3 weeks ago

A recent change to pytubefix includes debug information with the exception message. This should either be optional, logged or removed.

IMHO the new style violates the intended purpose of exception messages. I realize that this is being used for debugging purposes but there are established practices such as logging that are much better and preferred in almost all cases. The new change breaks my software which displays formatted information messages on errors.

Note: PEP 678 - Enriching Exceptions with Notes: This PEP introduced the add_note() method to add extra information to an exception object, which can be useful for debugging.

add_note()

Example Problem Exception Message:

Error: xxxxxxxxxxx requires login to view, YouTube reason: Please sign in

OS: win32 Python: 3.12.2 (tags/v3.12.2:6abddd9, Feb 6 2024, 21:26:36) [MSC v.1937 64 bit (AMD64)] Pytubefix: 7.3.0

felipeucelli commented 3 weeks ago

You're right, these messages can break some systems, but I noticed that add_note() was only added in Python 3.11 and some people are limited to older versions of Python due to the operating system (my country for example has several people who still use Windows 7). What do you think about passing this information to logger.debug() ? So in the issue template the user would be instructed to provide the debug log.

JuanBindez commented 3 weeks ago

A recent change to pytubefix includes debug information with the exception message. This should either be optional, logged or removed.

IMHO the new style violates the intended purpose of exception messages. I realize that this is being used for debugging purposes but there are established practices such as logging that are much better and preferred in almost all cases. The new change breaks my software which displays formatted information messages on errors.

Note: PEP 678 - Enriching Exceptions with Notes: This PEP introduced the add_note() method to add extra information to an exception object, which can be useful for debugging.

add_note()

Example Problem Exception Message:

Error: xxxxxxxxxxx requires login to view, YouTube reason: Please sign in

OS: win32 Python: 3.12.2 (tags/v3.12.2:6abddd9, Feb 6 2024, 21:26:36) [MSC v.1937 64 bit (AMD64)] Pytubefix: 7.3.0

I understand perfectly, exactly with this in mind I will remove it in the next versions and I will not be able to include PEP 678 because it is newer Python 3.11

JuanBindez commented 3 weeks ago

and it's not helping much either because there are some exceptions that aren't from pytubefix and it ends up falling into the same problem

JuanBindez commented 3 weeks ago

I made a mistake, I already fixed it -> https://github.com/JuanBindez/pytubefix/commit/5be3d633efaaf72a946d465f283271d66220560c

JuanBindez commented 3 weeks ago

see if it turned out interesting https://github.com/JuanBindez/pytubefix/pull/269

jhanley-com commented 3 weeks ago

My preference is that you create a logfile. Something like pytube.log. Then you can include a lot more information in the logs that will help you isolate problems.

Consider allowing the developer to specify the logfile name and loglevel. This could be an additional optional parameters to YouTube() and Playlist()

JuanBindez commented 3 weeks ago

in fact, normally those who create logs for their programs are the developers of their programs, in the case of the library I don't think it would be interesting to create a log, I believe that with this 'info' function it already makes things easier with what we need, if I don't If you understand it correctly, explain it better and feel free to make a PR.

JuanBindez commented 3 weeks ago

In the case of the info function, people who are used to opening issues just import info and make a print, this already helps us, now in your case it seems to be private

jhanley-com commented 3 weeks ago

I disagree with the closing of this issue. It is not resolved in my opinion nor are the arguments persuasive.

NannoSilver commented 3 weeks ago

I am running Pytubefix 7.3.1. Seems @JuanBindez removed the additional information that can mess the logs.

Who needs the Pytubefix version together with the log message can add manually pytubefix.__version__ that is very easy. Here is an example tested on Python 3.9:

import pytubefix

print(pytubefix.__version__)
JuanBindez commented 3 weeks ago

I am running Pytubefix 7.3.1. Seems @JuanBindez removed the additional information that can mess the logs.

Who needs the Pytubefix version together with the log message can add manually pytubefix.__version__ that is very easy. Here is an example tested on Python 3.9:

import pytubefix

print(pytubefix.__version__)

see #269

JuanBindez commented 3 weeks ago

I unified this information to make things easier, and now it doesn't get in the way at all

jhanley-com commented 3 weeks ago

I will try again with version 7.3.1

Thanks everyone.