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

should catch urllib2.URLError exceptions instead of crashing #1194

Closed
mschwendt opened this issue Aug 6, 2013 · 6 comments
Closed

should catch urllib2.URLError exceptions instead of crashing #1194

mschwendt opened this issue Aug 6, 2013 · 6 comments
Labels

Comments

@mschwendt
Copy link

@mschwendt mschwendt commented Aug 6, 2013

https://bugzilla.redhat.com/917380#c6

$ rpm -q youtube-dl
youtube-dl-2013.08.02-1.fc19.noarch
$ youtube-dl http://abc
Traceback (most recent call last):
[...]
urllib2.URLError: <urlopen error [Errno -2] Name or service not known>

The Python traceback triggers automatic bug reporting tools, such as ABRT, and users will report such crashes.

@cicku
Copy link

@cicku cicku commented Aug 7, 2013

I think youtube-dl should handle such error before downloading.

@phihag
Copy link
Contributor

@phihag phihag commented Feb 9, 2015

Thank you for the report, and sorry for the delay. For quite a while now, youtube-dl will only print a summary of the error by default:

$ youtube-dl http://abc
[generic] abc: Requesting header
WARNING: Could not send HEAD request to http://abc: <urlopen error [Errno -2] Name or service not known>
[generic] abc: Downloading webpage
ERROR: Unable to download webpage: <urlopen error [Errno -2] Name or service not known> (caused by URLError(gaierror(-2, 'Name or service not known'),))

This should not trigger abrt. If you pass in -v, you'll get a complete stack trace, but that's the whole point of -v.

I do think that the current situation works well, but then again, I don't use abrt, and our users don't report abrt bug reports here. Therefore, I am closing this entry, but feel free to comment and we will reopen it if necessary.

If you get reports from -v as well, is there a way we can tell abrt that this is not an unforeseen error in youtube-dl? Would it be sufficient to post additional text after the stacktrace?

@phihag phihag closed this Feb 9, 2015
@mschwendt
Copy link
Author

@mschwendt mschwendt commented Feb 9, 2015

The tracebacks at http://bugz.fedoraproject.org/youtube-dl all seem to without -v.

Error handling and sanity checking of input/runtime data is a good thing.

@phihag
Copy link
Contributor

@phihag phihag commented Feb 10, 2015

https://bugzilla.redhat.com/show_bug.cgi?id=1045282 should be reproducible with youtube-dl http://localhost:1/. Nowaydays that shows up as

[generic] localhost:1: Requesting header
WARNING: Could not send HEAD request to http://localhost:1/: <urlopen error [Errno 111] Connection refused>
[generic] localhost:1: Downloading webpage
ERROR: Unable to download webpage: <urlopen error [Errno 111] Connection refused> (caused by URLError(error(111, 'Connection refused'),))

https://bugzilla.redhat.com/show_bug.cgi?id=1090230 should be reproducible by running

import http.server
class MyHandler(http.server.BaseHTTPRequestHandler):
    handle_one_request = lambda *args: 0
http.server.HTTPServer(('', 1201), MyHandler).serve_forever()

in the background and then youtube-dl http://localhost:1201/. Nowadays that shows up as

[generic] localhost:1201: Requesting header
WARNING: Could not send HEAD request to http://localhost:1201/: ''
[generic] localhost:1201: Downloading webpage
ERROR: Unable to download webpage: '' (caused by BadStatusLine("''",)); please report this issue on https://yt-dl.org/bug . Make sure you are using the latest version; see  https://yt-dl.org/update  on how to update. Be sure to call youtube-dl with the --verbose flag and include its complete output.

https://bugzilla.redhat.com/show_bug.cgi?id=1098690 , https://bugzilla.redhat.com/show_bug.cgi?id=1122959 , https://bugzilla.redhat.com/show_bug.cgi?id=1176320 seem to be a condition where sys.stderr.write(b'something') fails. This seems to be worthy of a bug trace by any means; without the trace I'd be even more surprised about what is happening here.

https://bugzilla.redhat.com/show_bug.cgi?id=1115734 is not reproducable in a current version of youtube-dl; it (i.e. https://secure.brightcove.com/services/viewer/federated_f9?) produces a rather nice output:

ERROR: Cannot find playerKey= variable. Did you forget quotes in a shell invocation?

https://bugzilla.redhat.com/show_bug.cgi?id=1159600 seems to have been fixed since reporting; the download works fine for me now. Just to be certain, I've changed the offending code so that we get a more usable trace should a similar problem occur again.

https://bugzilla.redhat.com/show_bug.cgi?id=1181110 has been fixed in youtube-dl 2015.01.01. In any case, throwing an error seems fine here.

https://bugzilla.redhat.com/show_bug.cgi?id=1135594 seems to be a true network error. It have not found a way for it to occur under Python 2.7.9 though, and the IncompleteRead error is already handled quite nicely by us. In any case, I've added explicit handling in youtube-dl 2015.02.10 (dd1ab5e70c0024b94a37358d90791fe8949727b2).

https://bugzilla.redhat.com/show_bug.cgi?id=1002310 seems to have been resolved (the report author commented so) and just needs to be closed. In any case, this again seems to have been a real bug in youtube-dl for which a bug report was warranted.

https://bugzilla.redhat.com/show_bug.cgi?id=1005359 is in a very similar situation: the comments make it clear that it should have been closed, and the actual error was in a custom build of youtube-dl anyways. And again, a bug report is definitly warranted here.

https://bugzilla.redhat.com/show_bug.cgi?id=1093517 is someone who calls youtube-dl with a BOM in the URL, an unforeseen situation which was handled incorrectly beforehand. Again, I believe this warranted a stack trace. In any case, version 2015.02.10.1 adds some rather nice handling of this case:

$ youtube-dl test:unicodebom
[TestURL] Test URL: http://www.youtube.com/watch?v=BaW_jenozKc
WARNING: [UnicodeBOM] Your URL starts with a Byte Order Mark (BOM). Removing the BOM and looking for "http://www.youtube.com/watch?v=BaW_jenozKc" ...

Summarizingly, I do believe that not only most of the reports in the current list can be closed, but also that they fall into one of two categories, namely actual bugs that have been fixed in youtube-dl, and rather surprising environments, which are unquestionably worthy of investigation and a bug report.
Can you name a single invocation or report that shows the problems you want to fix with this bug issue, instead of "all of them"?


I disagree that we should do more to handle errors. In many cases, an extractor fetches JSON and simply goes over keys. It seems far more Pythonic to simply let the KeyError bubble up and wrap it (so it does not show up in abrt without -v).

@mschwendt
Copy link
Author

@mschwendt mschwendt commented Feb 10, 2015

Thanks for the many comments.

Can you name a single invocation or report that shows the problems you
want to fix with this bug issue, instead of "all of them"?

I pointed at the list of bugs users have decided to report (mostly because of ABRT intercepting the crash) to give examples of what issues they run into. The users seems to be overwhelmed with the traceback output and believe the program to be faulty in some way. Else they would not report/forward such tracebacks, but read them and draw their own conclusions. It is also difficult to decide for users whether to report to Fedora (bugs can be specific to it) or whether to report directly to youtube-dl developers. ABRT makes it too easy to report something to Fedora with a few mouse-clicks and be done. Where a package maintainer doesn't examine those incoming reports, they will be useless, because not even a valid traceback would be forwarded upstream.

Again, thanks for the comments. I'll notify all youtube-dl maintainers here at Fedora about them.

@tyll
Copy link
Contributor

@tyll tyll commented Feb 10, 2015

Thank you all very much for helping to triage the bug reports from Red Hat's bugzilla.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.