Fatal error messages should remind me about 429 errors. #32706
Comments
|
This seems like a case where the non-fatal error shouldn't have been, but you'd have to provide more details, as in the bug report template(s). |
|
Not a bug because the problem is that my proxy's IP is temporarily blacklisted due to too other users' behaviors. It works via another proxy. Verbose logThe so probably |
|
The ITV extractor probably fails anyway, and the default You would have to maintain a list of (host, port) that had experienced 429. A good place to set that up might be in |
Have you seen the verbose log in the details tag? I wish GitHub would make their "click to open" arrow a bit more visible. Or what more log would you need? |
|
AHA, sorry. In this case, the YT webpage failed with 429 but yt-dl was able to fetch the JSON which is supposed to be enough to get the video, though not to resolve the n-sig, or get the full metadata. Which actual code are you running there? The logic was updated in the most recent nightly. One possibility, without checking, is that the failure during n-sig was introduced, or not removed; another is that it more properly proceeds with a warning. |
iirc I had pulled before reproducing for the log. If it happens again, what version information should I collect? |
|
Yes, so I think this patch should be applied to treat the first failure in the routine in the same way as the later cases: --- old/youtube-dl/youtube_dl/extractor/youtube.py
+++ new/youtube-dl/youtube_dl/extractor/youtube.py
@@ -1631,7 +1631,9 @@
def _decrypt_nsig(self, n, video_id, player_url):
"""Turn the encrypted n field into a working signature"""
if player_url is None:
- raise ExtractorError('Cannot decrypt nsig without player_url')
+ self.report_warning(
+ 'Cannot decrypt nsig without player_url', video_id)
+ return
try:
jsi, player_id, func_code = self._extract_n_function_code(video_id, player_url)
@@ -1646,11 +1648,9 @@
ret = extract_nsig(jsi, func_code)(n)
except JSInterpreter.Exception as e:
self.report_warning(
- '%s (%s %s)' % (
- self.__ie_msg(
- 'Unable to decode n-parameter: download likely to be throttled'),
- error_to_compat_str(e),
- traceback.format_exc()))
+ 'Unable to decode n-parameter: download likely to be throttled (%s %s)' % (
+ error_to_compat_str(e), traceback.format_exc()),
+ video_id)
return
self.write_debug('Decrypted nsig {0} => {1}'.format(n, ret))The second hunk is an unrelated clean-up! |
|
Thanks! I will try it. |
|
The patch works nicely. It seems to replace the one specific fatal error above with downloading anyway: Which is good and we should get this into master. Thanks! However, now without that error, I can't tell whether it would achieve the actual feature requested above. Would it annotate any other fatal errors that may occurr after a 429 error? |
|
Such cases should be resolved in the same way, if they occur: (a) if the 429 will prevent extraction it should be an error; (b) if not it should be a warning, unless (c) exceptionally, the fallback tactics result in the same quality of extraction, where it might just be ignored or logged for debug. The problem here was poor logic such that although (b) should have applied the extraction failed anyway. |
|
I'm not entirely sure I understand it but I think you mean that even with a 429 error, fatal errors should still be reported as a bug, so the message saying so is actually correct. In that case, my feature request was misguided and you can just close it. |
|
Yes, that was my point. The issue could be left open until I push the change (actually there's a slicker version now). |
|
I have a new case, which looks a LOT like yet another dupe of 429 + It's not about the patch either, as I was able to download the same video earlier. Verbose log (click to open) |
|
Well, #23638 applies. In this case the JSON contains streaming formats but they only contain encrypted media links ("signature cipher"). The extractor needs the Would that be better? We now have the |
Yeah I read that, and am rather confident that the issue is redundantly not worth reporting, especially due to lack of reproducible console access. Which means we probably have a case where my original feature request is useful.
Probably not. "Some formats may be missing" to me sounds like "YouTube doesn't like you(r proxy)", for which my default route of action would be to use the next proxy in my list. Thus, my use is actually twofold:
|
Checklist
Description
I put too much effort into trying to report what seemed to be a bug, only to then discover in the log that there was a "HTTP Error 429: Too Many Requests" earlier. Can we make it so that any non-fatal 429 sets a kind of "tainted" flag and have all subsequent fatal errors start their message with something like "This might be caused by an earlier error: HTTP Error 429: Too Many Requests –"? That way I'd instantly see I can ignore the "please report this bug" part.
The text was updated successfully, but these errors were encountered: