From f27ec53c088010e87dc36f2c9688e1f3821a572a Mon Sep 17 00:00:00 2001 From: Stypox Date: Thu, 4 Sep 2025 22:31:24 +0200 Subject: [PATCH] Even more centralized error handling in ErrorInfo --- .../org/schabi/newpipe/RouterActivity.java | 31 ++-- .../newpipe/download/DownloadDialog.java | 9 +- .../newpipe/error/AcraReportSender.java | 2 +- .../org/schabi/newpipe/error/ErrorInfo.kt | 155 +++++++++++++----- .../schabi/newpipe/error/ErrorPanelHelper.kt | 60 ++----- .../fragments/detail/VideoDetailFragment.java | 6 +- .../fragments/list/BaseListInfoFragment.java | 8 +- .../list/channel/ChannelFragment.java | 2 +- .../fragments/list/search/SearchFragment.java | 24 ++- .../SubscriptionsImportFragment.java | 2 +- .../org/schabi/newpipe/player/Player.java | 5 +- .../schabi/newpipe/util/SparseItemUtil.java | 2 +- .../util/text/InternalUrlsHandler.java | 4 +- .../giga/ui/adapter/MissionAdapter.java | 2 +- 14 files changed, 178 insertions(+), 134 deletions(-) diff --git a/app/src/main/java/org/schabi/newpipe/RouterActivity.java b/app/src/main/java/org/schabi/newpipe/RouterActivity.java index cb7ea3dd7..50639c5ae 100644 --- a/app/src/main/java/org/schabi/newpipe/RouterActivity.java +++ b/app/src/main/java/org/schabi/newpipe/RouterActivity.java @@ -58,10 +58,7 @@ import org.schabi.newpipe.extractor.NewPipe; import org.schabi.newpipe.extractor.StreamingService; import org.schabi.newpipe.extractor.StreamingService.LinkType; import org.schabi.newpipe.extractor.channel.ChannelInfo; -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.ReCaptchaException; import org.schabi.newpipe.extractor.linkhandler.ListLinkHandler; import org.schabi.newpipe.extractor.playlist.PlaylistInfo; import org.schabi.newpipe.extractor.stream.StreamInfo; @@ -253,7 +250,8 @@ public class RouterActivity extends AppCompatActivity { showUnsupportedUrlDialog(url); } }, throwable -> handleError(this, new ErrorInfo(throwable, - UserAction.SHARE_TO_NEWPIPE, "Getting service from url: " + url)))); + UserAction.SHARE_TO_NEWPIPE, "Getting service from url: " + url, + null, url)))); } /** @@ -262,23 +260,19 @@ public class RouterActivity extends AppCompatActivity { * @param errorInfo the error information */ private static void handleError(final Context context, final ErrorInfo errorInfo) { - if (errorInfo.getThrowable() != null) { - errorInfo.getThrowable().printStackTrace(); - } - - if (errorInfo.getThrowable() instanceof ReCaptchaException) { + if (errorInfo.getRecaptchaUrl() != null) { Toast.makeText(context, R.string.recaptcha_request_toast, Toast.LENGTH_LONG).show(); // Starting ReCaptcha Challenge Activity final Intent intent = new Intent(context, ReCaptchaActivity.class); intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK); + intent.putExtra(ReCaptchaActivity.RECAPTCHA_URL_EXTRA, errorInfo.getRecaptchaUrl()); context.startActivity(intent); - } else if (errorInfo.getThrowable() instanceof ContentNotAvailableException - || errorInfo.getThrowable() instanceof ContentNotSupportedException) { + } else if (errorInfo.isReportable()) { + ErrorUtil.createNotification(context, errorInfo); + } else { // this exception does not usually indicate a problem that should be reported, // so just show a toast instead of the notification Toast.makeText(context, errorInfo.getMessage(context), Toast.LENGTH_LONG).show(); - } else { - ErrorUtil.createNotification(context, errorInfo); } if (context instanceof RouterActivity) { @@ -641,7 +635,8 @@ public class RouterActivity extends AppCompatActivity { startActivity(intent); finish(); }, throwable -> handleError(this, new ErrorInfo(throwable, - UserAction.SHARE_TO_NEWPIPE, "Starting info activity: " + currentUrl))) + UserAction.SHARE_TO_NEWPIPE, "Starting info activity: " + currentUrl, + null, currentUrl))) ); return; } @@ -828,10 +823,10 @@ public class RouterActivity extends AppCompatActivity { }) )), throwable -> runOnVisible(ctx -> handleError(ctx, new ErrorInfo( - throwable, - UserAction.REQUESTED_STREAM, + throwable, UserAction.REQUESTED_STREAM, "Tried to add " + currentUrl + " to a playlist", - ((RouterActivity) ctx).currentService.getServiceId()) + ((RouterActivity) ctx).currentService.getServiceId(), + currentUrl) )) ) ); @@ -971,7 +966,7 @@ public class RouterActivity extends AppCompatActivity { } }, throwable -> handleError(this, new ErrorInfo(throwable, finalUserAction, choice.url + " opened with " + choice.playerChoice, - choice.serviceId))); + choice.serviceId, choice.url))); } } diff --git a/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java b/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java index 003aa5893..0857fa339 100644 --- a/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java +++ b/app/src/main/java/org/schabi/newpipe/download/DownloadDialog.java @@ -389,8 +389,7 @@ public class DownloadDialog extends DialogFragment } }, throwable -> ErrorUtil.showSnackbar(context, new ErrorInfo(throwable, UserAction.DOWNLOAD_OPEN_DIALOG, - "Downloading video stream size", - currentInfo.getServiceId())))); + "Downloading video stream size", currentInfo)))); disposables.add(StreamInfoWrapper.fetchMoreInfoForWrapper(getWrappedAudioStreams()) .subscribe(result -> { if (dialogBinding.videoAudioGroup.getCheckedRadioButtonId() @@ -399,8 +398,7 @@ public class DownloadDialog extends DialogFragment } }, throwable -> ErrorUtil.showSnackbar(context, new ErrorInfo(throwable, UserAction.DOWNLOAD_OPEN_DIALOG, - "Downloading audio stream size", - currentInfo.getServiceId())))); + "Downloading audio stream size", currentInfo)))); disposables.add(StreamInfoWrapper.fetchMoreInfoForWrapper(wrappedSubtitleStreams) .subscribe(result -> { if (dialogBinding.videoAudioGroup.getCheckedRadioButtonId() @@ -409,8 +407,7 @@ public class DownloadDialog extends DialogFragment } }, throwable -> ErrorUtil.showSnackbar(context, new ErrorInfo(throwable, UserAction.DOWNLOAD_OPEN_DIALOG, - "Downloading subtitle stream size", - currentInfo.getServiceId())))); + "Downloading subtitle stream size", currentInfo)))); } private void setupAudioTrackSpinner() { diff --git a/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java b/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java index 8876a66e4..90d8f4797 100644 --- a/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java +++ b/app/src/main/java/org/schabi/newpipe/error/AcraReportSender.java @@ -36,8 +36,8 @@ public class AcraReportSender implements ReportSender { ErrorUtil.openActivity(context, new ErrorInfo( new String[]{report.getString(ReportField.STACK_TRACE)}, UserAction.UI_ERROR, - null, "ACRA report", + null, R.string.app_ui_crash)); } } diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt index bac294d0f..609fbb336 100644 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorInfo.kt @@ -7,7 +7,6 @@ import androidx.core.content.ContextCompat 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 @@ -29,70 +28,108 @@ import org.schabi.newpipe.extractor.exceptions.YoutubeMusicPremiumContentExcepti import org.schabi.newpipe.ktx.isNetworkRelated import org.schabi.newpipe.player.mediasource.FailedMediaSource import org.schabi.newpipe.player.resolver.PlaybackResolver +import java.net.UnknownHostException +/** + * An error has occurred in the app. This class contains plain old parcelable data that can be used + * to report the error and to show it to the user along with correct action buttons. + */ @Parcelize class ErrorInfo private constructor( val stackTraces: Array, val userAction: UserAction, - val serviceId: Int?, val request: String, + val serviceId: Int?, private val message: ErrorMessage, + /** + * If `true`, a report button will be shown for this error. Otherwise the error is not something + * that can really be reported (e.g. a network issue, or content not being available at all). + */ + val isReportable: Boolean, + /** + * If `true`, the process causing this error can be retried, otherwise not. + */ + val isRetryable: Boolean, + /** + * If present, indicates that the exception was a ReCaptchaException, and this is the URL + * provided by the service that can be used to solve the ReCaptcha challenge. + */ + val recaptchaUrl: String?, + /** + * If present, this resource can alternatively be opened in browser (useful if NewPipe is + * badly broken). + */ + val openInBrowserUrl: String?, ) : Parcelable { - // no need to store throwable, all data for report is in other variables - // also, the throwable might not be serializable, see TeamNewPipe/NewPipe#7302 - @IgnoredOnParcel - var throwable: Throwable? = null - - private constructor( + @JvmOverloads + constructor( throwable: Throwable, userAction: UserAction, - serviceId: Int?, - request: String + request: String, + serviceId: Int? = null, + openInBrowserUrl: String? = null, ) : this( throwableToStringList(throwable), userAction, - serviceId, request, - getMessage(throwable, userAction, serviceId) - ) { - this.throwable = throwable - } + serviceId, + getMessage(throwable, userAction, serviceId), + isReportable(throwable), + isRetryable(throwable), + (throwable as? ReCaptchaException)?.url, + openInBrowserUrl, + ) - private constructor( - throwable: List, + @JvmOverloads + constructor( + throwables: List, userAction: UserAction, - serviceId: Int?, - request: String + request: String, + serviceId: Int? = null, + openInBrowserUrl: String? = null, ) : this( - throwableListToStringList(throwable), + throwableListToStringList(throwables), userAction, - serviceId, request, - getMessage(throwable.firstOrNull(), userAction, serviceId) - ) { - this.throwable = throwable.firstOrNull() - } + serviceId, + getMessage(throwables.firstOrNull(), userAction, serviceId), + throwables.any(::isReportable), + throwables.isEmpty() || throwables.any(::isRetryable), + throwables.firstNotNullOfOrNull { it as? ReCaptchaException }?.url, + openInBrowserUrl, + ) - // constructor to manually build ErrorInfo - constructor(stackTraces: Array, userAction: UserAction, serviceId: Int?, request: String, @StringRes message: Int) : - this(stackTraces, userAction, serviceId, request, ErrorMessage(message)) + // constructor to manually build ErrorInfo when no throwable is available + constructor( + stackTraces: Array, + userAction: UserAction, + request: String, + serviceId: Int?, + @StringRes message: Int + ) : + this( + stackTraces, userAction, request, serviceId, ErrorMessage(message), + true, false, null, null + ) - // constructors with single throwable - constructor(throwable: Throwable, userAction: UserAction, request: String) : - this(throwable, userAction, null, request) - constructor(throwable: Throwable, userAction: UserAction, request: String, serviceId: Int) : - this(throwable, userAction, serviceId, request) - constructor(throwable: Throwable, userAction: UserAction, request: String, info: Info?) : - this(throwable, userAction, info?.serviceId, request) + // constructor with only one throwable to extract service id and openInBrowserUrl from an Info + constructor( + throwable: Throwable, + userAction: UserAction, + request: String, + info: Info?, + ) : + this(throwable, userAction, request, info?.serviceId, info?.url) - // constructors with list of throwables - constructor(throwable: List, userAction: UserAction, request: String) : - this(throwable, userAction, null, request) - constructor(throwable: List, userAction: UserAction, request: String, serviceId: Int) : - this(throwable, userAction, serviceId, request) - constructor(throwable: List, userAction: UserAction, request: String, info: Info?) : - this(throwable, userAction, info?.serviceId, request) + // constructor with multiple throwables to extract service id and openInBrowserUrl from an Info + constructor( + throwables: List, + userAction: UserAction, + request: String, + info: Info?, + ) : + this(throwables, userAction, request, info?.serviceId, info?.url) fun getServiceName(): String { return getServiceName(serviceId) @@ -205,8 +242,7 @@ class ErrorInfo private constructor( // other extractor exceptions throwable is ContentNotSupportedException -> ErrorMessage(R.string.content_not_supported) - // ReCaptchas should have already been handled elsewhere, - // but return an error message here just in case + // ReCaptchas will be handled in a special way anyway throwable is ReCaptchaException -> ErrorMessage(R.string.recaptcha_request_toast) // test this at the end as many exceptions could be a subclass of IOException @@ -234,5 +270,36 @@ class ErrorInfo private constructor( ErrorMessage(R.string.error_snackbar_message) } } + + fun isReportable(throwable: Throwable?): Boolean { + return when (throwable) { + // we don't have an exception, so this is a manually built error, which likely + // indicates that it's important and is thus reportable + null -> true + // the service explicitly said that content is not available (e.g. age restrictions, + // video deleted, etc.), there is no use in letting users report it + is ContentNotAvailableException -> false + // we know the content is not supported, no need to let the user report it + is ContentNotSupportedException -> false + // happens often when there is no internet connection; we don't use + // `throwable.isNetworkRelated` since any `IOException` would make that function + // return true, but not all `IOException`s are network related + is UnknownHostException -> false + // by default, this is an unexpected exception, which the user could report + else -> true + } + } + + fun isRetryable(throwable: Throwable?): Boolean { + return when (throwable) { + // we know the content is not available, retrying won't help + is ContentNotAvailableException -> false + // we know the content is not supported, retrying won't help + is ContentNotSupportedException -> false + // by default (including if throwable is null), enable retrying (though the retry + // button will be shown only if a way to perform the retry is implemented) + else -> true + } + } } } diff --git a/app/src/main/java/org/schabi/newpipe/error/ErrorPanelHelper.kt b/app/src/main/java/org/schabi/newpipe/error/ErrorPanelHelper.kt index 959759127..4ec5f58c3 100644 --- a/app/src/main/java/org/schabi/newpipe/error/ErrorPanelHelper.kt +++ b/app/src/main/java/org/schabi/newpipe/error/ErrorPanelHelper.kt @@ -2,7 +2,6 @@ package org.schabi.newpipe.error import android.content.Context import android.content.Intent -import android.util.Log import android.view.View import android.widget.Button import android.widget.TextView @@ -14,11 +13,7 @@ import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers import io.reactivex.rxjava3.disposables.Disposable import org.schabi.newpipe.MainActivity import org.schabi.newpipe.R -import org.schabi.newpipe.extractor.exceptions.ContentNotAvailableException -import org.schabi.newpipe.extractor.exceptions.ContentNotSupportedException -import org.schabi.newpipe.extractor.exceptions.ReCaptchaException import org.schabi.newpipe.ktx.animate -import org.schabi.newpipe.ktx.isInterruptedCaused import org.schabi.newpipe.util.external_communication.ShareUtils import java.util.concurrent.TimeUnit @@ -68,50 +63,32 @@ class ErrorPanelHelper( } fun showError(errorInfo: ErrorInfo) { - - if (errorInfo.throwable != null && errorInfo.throwable!!.isInterruptedCaused) { - if (DEBUG) { - Log.w(TAG, "onError() isInterruptedCaused! = [$errorInfo.throwable]") - } - return - } - ensureDefaultVisibility() + errorTextView.text = errorInfo.getMessage(context) - if (errorInfo.throwable is ReCaptchaException) { - errorTextView.setText(R.string.recaptcha_request_toast) - - showAndSetErrorButtonAction( - R.string.recaptcha_solve - ) { + if (errorInfo.recaptchaUrl != null) { + showAndSetErrorButtonAction(R.string.recaptcha_solve) { // Starting ReCaptcha Challenge Activity val intent = Intent(context, ReCaptchaActivity::class.java) - intent.putExtra( - ReCaptchaActivity.RECAPTCHA_URL_EXTRA, - (errorInfo.throwable as ReCaptchaException).url - ) + intent.putExtra(ReCaptchaActivity.RECAPTCHA_URL_EXTRA, errorInfo.recaptchaUrl) fragment.startActivityForResult(intent, ReCaptchaActivity.RECAPTCHA_REQUEST) errorActionButton.setOnClickListener(null) } - - errorRetryButton.isVisible = retryShouldBeShown - showAndSetOpenInBrowserButtonAction(errorInfo) - } else { - showAndSetErrorButtonAction( - R.string.error_snackbar_action - ) { + } else if (errorInfo.isReportable) { + showAndSetErrorButtonAction(R.string.error_snackbar_action) { ErrorUtil.openActivity(context, errorInfo) } + } - errorTextView.text = errorInfo.getMessage(context) + if (errorInfo.isRetryable) { + errorRetryButton.isVisible = retryShouldBeShown + } - if (errorInfo.throwable !is ContentNotAvailableException && - errorInfo.throwable !is ContentNotSupportedException - ) { - // show retry button only for content which is not unavailable or unsupported - errorRetryButton.isVisible = retryShouldBeShown + if (errorInfo.openInBrowserUrl != null) { + errorOpenInBrowserButton.isVisible = true + errorOpenInBrowserButton.setOnClickListener { + ShareUtils.openUrlInBrowser(context, errorInfo.openInBrowserUrl) } - showAndSetOpenInBrowserButtonAction(errorInfo) } setRootVisible() @@ -129,15 +106,6 @@ class ErrorPanelHelper( errorActionButton.setOnClickListener(listener) } - fun showAndSetOpenInBrowserButtonAction( - errorInfo: ErrorInfo - ) { - errorOpenInBrowserButton.isVisible = true - errorOpenInBrowserButton.setOnClickListener { - ShareUtils.openUrlInBrowser(context, errorInfo.request) - } - } - fun showTextError(errorString: String) { ensureDefaultVisibility() diff --git a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java index b46b0c708..342333aa5 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/detail/VideoDetailFragment.java @@ -876,7 +876,7 @@ public final class VideoDetailFragment } } }, throwable -> showError(new ErrorInfo(throwable, UserAction.REQUESTED_STREAM, - url == null ? "no url" : url, serviceId))); + url == null ? "no url" : url, serviceId, url))); } /*////////////////////////////////////////////////////////////////////////// @@ -1593,8 +1593,8 @@ public final class VideoDetailFragment } if (!info.getErrors().isEmpty()) { - showSnackBarError(new ErrorInfo(info.getErrors(), - UserAction.REQUESTED_STREAM, info.getUrl(), info)); + showSnackBarError(new ErrorInfo(info.getErrors(), UserAction.REQUESTED_STREAM, + "Some info not extracted: " + info.getUrl(), info)); } } diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java index 7f594734a..848dfe6f5 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/BaseListInfoFragment.java @@ -153,7 +153,7 @@ public abstract class BaseListInfoFragment showError(new ErrorInfo(throwable, errorUserAction, - "Start loading: " + url, serviceId))); + "Start loading: " + url, serviceId, url))); } /** @@ -184,7 +184,7 @@ public abstract class BaseListInfoFragment dynamicallyShowErrorPanelOrSnackbar(new ErrorInfo(throwable, - errorUserAction, "Loading more items: " + url, serviceId))); + errorUserAction, "Loading more items: " + url, serviceId, url))); } private void forbidDownwardFocusScroll() { @@ -210,7 +210,7 @@ public abstract class BaseListInfoFragment isLoading.set(false); handleResult(result); }, throwable -> showError(new ErrorInfo(throwable, UserAction.REQUESTED_CHANNEL, - url == null ? "No URL" : url, serviceId))); + url == null ? "No URL" : url, serviceId, url))); } @Override diff --git a/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java b/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java index cea06b942..8cb5f6497 100644 --- a/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java +++ b/app/src/main/java/org/schabi/newpipe/fragments/list/search/SearchFragment.java @@ -54,6 +54,7 @@ import org.schabi.newpipe.extractor.MetaInfo; import org.schabi.newpipe.extractor.NewPipe; import org.schabi.newpipe.extractor.Page; import org.schabi.newpipe.extractor.StreamingService; +import org.schabi.newpipe.extractor.exceptions.ParsingException; import org.schabi.newpipe.extractor.search.SearchExtractor; import org.schabi.newpipe.extractor.search.SearchInfo; import org.schabi.newpipe.extractor.services.peertube.linkHandler.PeertubeSearchQueryHandlerFactory; @@ -934,7 +935,21 @@ public class SearchFragment extends BaseListFragment ErrorUtil.createNotification(context, new ErrorInfo(throwable, UserAction.REQUESTED_STREAM, - "Loading stream info: " + url, serviceId) + "Loading stream info: " + url, serviceId, url) )); } } diff --git a/app/src/main/java/org/schabi/newpipe/util/text/InternalUrlsHandler.java b/app/src/main/java/org/schabi/newpipe/util/text/InternalUrlsHandler.java index a2743141b..2e4aa320f 100644 --- a/app/src/main/java/org/schabi/newpipe/util/text/InternalUrlsHandler.java +++ b/app/src/main/java/org/schabi/newpipe/util/text/InternalUrlsHandler.java @@ -160,10 +160,10 @@ public final class InternalUrlsHandler { final PlayQueue playQueue = new SinglePlayQueue(info, seconds * 1000L); NavigationHelper.playOnPopupPlayer(context, playQueue, false); }, throwable -> { - final var errorInfo = new ErrorInfo(throwable, UserAction.PLAY_ON_POPUP, url); // This will only show a snackbar if the passed context has a root view: // otherwise it will resort to showing a notification, so we are safe here. - ErrorUtil.showSnackbar(context, errorInfo); + ErrorUtil.showSnackbar(context, + new ErrorInfo(throwable, UserAction.PLAY_ON_POPUP, url, null, url)); })); return true; } diff --git a/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java b/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java index 31065d57c..f245b3dd9 100644 --- a/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java +++ b/app/src/main/java/us/shandian/giga/ui/adapter/MissionAdapter.java @@ -572,7 +572,7 @@ public class MissionAdapter extends Adapter implements Handler.Callb ErrorUtil.createNotification(mContext, new ErrorInfo(ErrorInfo.Companion.throwableToStringList(mission.errObject), action, - service, request.toString(), reason)); + request.toString(), service, reason)); } public void clearFinishedDownloads(boolean delete) {