-
-
Notifications
You must be signed in to change notification settings - Fork 3.5k
Add more specific error messages and deduplicate their handling #12477
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,15 +3,28 @@ package org.schabi.newpipe.error | |
| import android.os.Parcelable | ||
| import androidx.annotation.StringRes | ||
| import com.google.android.exoplayer2.ExoPlaybackException | ||
| import com.google.android.exoplayer2.upstream.HttpDataSource | ||
| import com.google.android.exoplayer2.upstream.Loader | ||
| import kotlinx.parcelize.IgnoredOnParcel | ||
| import kotlinx.parcelize.Parcelize | ||
| import org.schabi.newpipe.R | ||
| import org.schabi.newpipe.extractor.Info | ||
| import org.schabi.newpipe.extractor.exceptions.AccountTerminatedException | ||
| import org.schabi.newpipe.extractor.exceptions.AgeRestrictedContentException | ||
| import org.schabi.newpipe.extractor.exceptions.ContentNotAvailableException | ||
| import org.schabi.newpipe.extractor.exceptions.ContentNotSupportedException | ||
| import org.schabi.newpipe.extractor.exceptions.ExtractionException | ||
| import org.schabi.newpipe.extractor.exceptions.GeographicRestrictionException | ||
| import org.schabi.newpipe.extractor.exceptions.PaidContentException | ||
| import org.schabi.newpipe.extractor.exceptions.PrivateContentException | ||
| import org.schabi.newpipe.extractor.exceptions.ReCaptchaException | ||
| import org.schabi.newpipe.extractor.exceptions.SoundCloudGoPlusContentException | ||
| import org.schabi.newpipe.extractor.exceptions.UnsupportedContentInCountryException | ||
| import org.schabi.newpipe.extractor.exceptions.YoutubeMusicPremiumContentException | ||
| import org.schabi.newpipe.extractor.exceptions.YoutubeSignInConfirmNotBotException | ||
| import org.schabi.newpipe.ktx.isNetworkRelated | ||
| import org.schabi.newpipe.player.mediasource.FailedMediaSource | ||
| import org.schabi.newpipe.player.resolver.PlaybackResolver | ||
| import org.schabi.newpipe.util.ServiceHelper | ||
|
|
||
| @Parcelize | ||
|
|
@@ -86,30 +99,57 @@ class ErrorInfo( | |
| if (info == null) SERVICE_NONE else ServiceHelper.getNameOfServiceById(info.serviceId) | ||
|
|
||
| @StringRes | ||
| private fun getMessageStringId( | ||
| fun getMessageStringId( | ||
| throwable: Throwable?, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be a stupid question but why are we catching If we encounter an
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For example when the app crashes and ACRA catches the throwable and starts the error report screen, that throwable could be anything. In other places... well yeah maybe we don't need to catch Throwables, but just to be sure sometimes we do. E.g. |
||
| action: UserAction | ||
| action: UserAction? | ||
| ): Int { | ||
| return when { | ||
| throwable is AccountTerminatedException -> R.string.account_terminated | ||
| throwable is ContentNotAvailableException -> R.string.content_not_available | ||
| throwable != null && throwable.isNetworkRelated -> R.string.network_error | ||
| throwable is ContentNotSupportedException -> R.string.content_not_supported | ||
| throwable is ExtractionException -> R.string.parsing_error | ||
| // player exceptions | ||
| // some may be IOException, so do these checks before isNetworkRelated! | ||
| throwable is ExoPlaybackException -> { | ||
| when (throwable.type) { | ||
| ExoPlaybackException.TYPE_SOURCE -> R.string.player_stream_failure | ||
| ExoPlaybackException.TYPE_UNEXPECTED -> R.string.player_recoverable_failure | ||
| val cause = throwable.cause | ||
| when { | ||
| cause is HttpDataSource.InvalidResponseCodeException && cause.responseCode == 403 -> R.string.player_error_403 | ||
| cause is Loader.UnexpectedLoaderException && cause.cause is ExtractionException -> getMessageStringId(throwable, action) | ||
| throwable.type == ExoPlaybackException.TYPE_SOURCE -> R.string.player_stream_failure | ||
| throwable.type == ExoPlaybackException.TYPE_UNEXPECTED -> R.string.player_recoverable_failure | ||
| else -> R.string.player_unrecoverable_failure | ||
| } | ||
| } | ||
| throwable is FailedMediaSource.FailedMediaSourceException -> getMessageStringId(throwable.cause, action) | ||
| throwable is PlaybackResolver.ResolverException -> R.string.player_stream_failure | ||
|
|
||
| // content not available exceptions | ||
| throwable is AccountTerminatedException -> R.string.account_terminated | ||
| throwable is AgeRestrictedContentException -> R.string.restricted_video_no_stream | ||
| throwable is GeographicRestrictionException -> R.string.georestricted_content | ||
| throwable is PaidContentException -> R.string.paid_content | ||
| throwable is PrivateContentException -> R.string.private_content | ||
| throwable is SoundCloudGoPlusContentException -> R.string.soundcloud_go_plus_content | ||
| throwable is UnsupportedContentInCountryException -> R.string.unsupported_content_in_country | ||
| throwable is YoutubeMusicPremiumContentException -> R.string.youtube_music_premium_content | ||
| throwable is YoutubeSignInConfirmNotBotException -> R.string.youtube_sign_in_confirm_not_bot_error | ||
| throwable is ContentNotAvailableException -> R.string.content_not_available | ||
|
|
||
| // other extractor exceptions | ||
| throwable is ContentNotSupportedException -> R.string.content_not_supported | ||
| // ReCaptchas should have already been handled elsewhere, | ||
| // but return an error message here just in case | ||
| throwable is ReCaptchaException -> R.string.recaptcha_request_toast | ||
| // test this at the end as many exceptions could be a subclass of IOException | ||
| throwable != null && throwable.isNetworkRelated -> R.string.network_error | ||
| // an extraction exception unrelated to the network | ||
| // is likely an issue with parsing the website | ||
| throwable is ExtractionException -> R.string.parsing_error | ||
|
|
||
| // user actions (in case the exception is null or unrecognizable) | ||
| action == UserAction.UI_ERROR -> R.string.app_ui_crash | ||
| action == UserAction.REQUESTED_COMMENTS -> R.string.error_unable_to_load_comments | ||
| action == UserAction.SUBSCRIPTION_CHANGE -> R.string.subscription_change_failed | ||
| action == UserAction.SUBSCRIPTION_UPDATE -> R.string.subscription_update_failed | ||
| action == UserAction.LOAD_IMAGE -> R.string.could_not_load_thumbnails | ||
| action == UserAction.DOWNLOAD_OPEN_DIALOG -> R.string.could_not_setup_download_menu | ||
| else -> R.string.general_error | ||
| else -> R.string.error_snackbar_message | ||
| } | ||
| } | ||
| } | ||
|
|
||
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.
This needs to be changed before merging