-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Fix detection of network related exceptions #3294
Fix detection of network related exceptions #3294
Conversation
This is one solution. However, with yt_new we also had this bug, and we decided that it's better to still just throw the IOException in NewPipeExtractor. We could reevaluate that choice though. |
Yes, throwing a IOException is not a problem, the problem is that some exceptions wrap it, and that can hide network errors here. The only time that a IOException would be throw, would be in a situation where the network can't make/complete the request. If it can make it at all, a response will always be returned, is up to the extractor code detect error situations (e.g. received a 4xx). Example (and a major source of reports) in the extractor where this error is hidden (where arguably it shouldn't). Any I/O error, like a slow network timing out that request, would result in a DecryptException. Fortunately, this is already handled correctly (i.e. not catching it) and only a few places there have this problem. This pull request would just make this check more thorough on this side, which help in some cases. |
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.
Ah, you're correct. I looked at the code and it looks good. You have to rebase though.
0fbd246
to
913796f
Compare
Just tested this a bit further and got one error which I did not expect. Log
|
@TobiGr: There it is, one more gotta catch 'em all situation. The method below also suffers from it. Now, the reason why only SoundCloud have this problem: the others implement the It actually returns a info object with the network error in its error list, that's why it's only a snackbar message. |
@mauriciocolli Thanks for taking a look at this. |
What is it?
Long description of the changes in your PR
I saw a few of these in some reports lately, because when a network exception occurs, some other method will sometimes wrap that exception and cause it not to be detected by the simple
instanceof
checks.This adds a class that checks whether some exception in the chain is of some network issue related type.
Fixes the following issue(s)
Testing apk
fix-network-issues-detection.apk.zip