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

Fix detection of network related exceptions #3294

Merged
merged 2 commits into from
Apr 10, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions app/src/main/java/org/schabi/newpipe/App.java
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import org.schabi.newpipe.report.ErrorActivity;
import org.schabi.newpipe.report.UserAction;
import org.schabi.newpipe.settings.SettingsActivity;
import org.schabi.newpipe.util.ExtractorHelper;
import org.schabi.newpipe.util.ExceptionUtils;
import org.schabi.newpipe.util.Localization;
import org.schabi.newpipe.util.ServiceHelper;
import org.schabi.newpipe.util.StateSaver;
Expand Down Expand Up @@ -173,7 +173,7 @@ public void accept(@NonNull final Throwable throwable) {

private boolean isThrowableIgnored(@NonNull final Throwable throwable) {
// Don't crash the application over a simple network problem
return ExtractorHelper.hasAssignableCauseThrowable(throwable,
return ExceptionUtils.hasAssignableCause(throwable,
// network api cancellation
IOException.class, SocketException.class,
// blocking code disposed
Expand All @@ -182,7 +182,7 @@ private boolean isThrowableIgnored(@NonNull final Throwable throwable) {

private boolean isThrowableCritical(@NonNull final Throwable throwable) {
// Though these exceptions cannot be ignored
return ExtractorHelper.hasAssignableCauseThrowable(throwable,
return ExceptionUtils.hasAssignableCause(throwable,
NullPointerException.class, IllegalArgumentException.class, // bug in app
OnErrorNotImplementedException.class, MissingBackpressureException.class,
IllegalStateException.class); // bug in operator
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,9 @@
import org.schabi.newpipe.extractor.exceptions.ReCaptchaException;
import org.schabi.newpipe.report.ErrorActivity;
import org.schabi.newpipe.report.UserAction;
import org.schabi.newpipe.util.ExtractorHelper;
import org.schabi.newpipe.util.ExceptionUtils;
import org.schabi.newpipe.util.InfoCache;

import java.io.IOException;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.TimeUnit;
Expand Down Expand Up @@ -200,7 +199,7 @@ protected boolean onError(final Throwable exception) {
return true;
}

if (ExtractorHelper.isInterruptedCaused(exception)) {
if (ExceptionUtils.isInterruptedCaused(exception)) {
if (DEBUG) {
Log.w(TAG, "onError() isInterruptedCaused! = [" + exception + "]");
}
Expand All @@ -213,7 +212,7 @@ protected boolean onError(final Throwable exception) {
} else if (exception instanceof ContentNotAvailableException) {
showError(getString(R.string.content_not_available), false);
return true;
} else if (exception instanceof IOException) {
} else if (ExceptionUtils.isNetworkRelated(exception)) {
showError(getString(R.string.network_error), true);
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,6 @@
import org.schabi.newpipe.util.NavigationHelper;
import org.schabi.newpipe.util.ServiceHelper;

import java.io.IOException;
import java.io.InterruptedIOException;
import java.net.SocketException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
Expand Down Expand Up @@ -763,12 +760,7 @@ private void initSuggestionObserver() {
if (listNotification.isOnNext()) {
handleSuggestions(listNotification.getValue());
} else if (listNotification.isOnError()) {
Throwable error = listNotification.getError();
if (!ExtractorHelper.hasAssignableCauseThrowable(error,
IOException.class, SocketException.class,
InterruptedException.class, InterruptedIOException.class)) {
onSuggestionError(error);
}
onSuggestionError(listNotification.getError());
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ import org.schabi.newpipe.local.feed.FeedDatabaseManager
import org.schabi.newpipe.local.feed.service.FeedEventManager.Event.*
import org.schabi.newpipe.local.feed.service.FeedEventManager.postEvent
import org.schabi.newpipe.local.subscription.SubscriptionManager
import org.schabi.newpipe.util.ExceptionUtils
import org.schabi.newpipe.util.ExtractorHelper
import java.io.IOException
import java.util.*
Expand Down Expand Up @@ -333,11 +334,12 @@ class FeedLoadService : Service() {
val cause = error.cause

when {
error is IOException -> throw error
cause is IOException -> throw cause

error is ReCaptchaException -> throw error
cause is ReCaptchaException -> throw cause

error is IOException -> throw error
cause is IOException -> throw cause
ExceptionUtils.isNetworkRelated(error) -> throw IOException(error)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@
import org.schabi.newpipe.local.subscription.SubscriptionManager;
import org.schabi.newpipe.report.ErrorActivity;
import org.schabi.newpipe.report.UserAction;
import org.schabi.newpipe.util.ExceptionUtils;

import java.io.FileNotFoundException;
import java.io.IOException;
import java.util.Collections;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;
Expand Down Expand Up @@ -227,7 +227,7 @@ protected String getErrorMessage(final Throwable error) {
message = getString(R.string.invalid_source);
} else if (error instanceof FileNotFoundException) {
message = getString(R.string.invalid_file);
} else if (error instanceof IOException) {
} else if (ExceptionUtils.isNetworkRelated(error)) {
message = getString(R.string.network_error);
}
return message;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import org.schabi.newpipe.extractor.channel.ChannelInfo;
import org.schabi.newpipe.extractor.subscription.SubscriptionItem;
import org.schabi.newpipe.util.Constants;
import org.schabi.newpipe.util.ExceptionUtils;
import org.schabi.newpipe.util.ExtractorHelper;

import java.io.File;
Expand Down Expand Up @@ -245,8 +246,10 @@ private Consumer<Notification<ChannelInfo>> getNotificationsConsumer() {
final Throwable cause = error.getCause();
if (error instanceof IOException) {
throw (IOException) error;
} else if (cause != null && cause instanceof IOException) {
} else if (cause instanceof IOException) {
throw (IOException) cause;
} else if (ExceptionUtils.isNetworkRelated(error)) {
throw new IOException(error);
}

eventListener.onItemCompleted("");
Expand Down
82 changes: 82 additions & 0 deletions app/src/main/java/org/schabi/newpipe/util/ExceptionUtils.kt
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
package org.schabi.newpipe.util

import java.io.IOException
import java.io.InterruptedIOException

class ExceptionUtils {
companion object {
/**
* @return if throwable is related to Interrupted exceptions, or one of its causes is.
*/
@JvmStatic
fun isInterruptedCaused(throwable: Throwable): Boolean {
return hasExactCause(throwable,
InterruptedIOException::class.java,
InterruptedException::class.java)
}

/**
* @return if throwable is related to network issues, or one of its causes is.
*/
@JvmStatic
fun isNetworkRelated(throwable: Throwable): Boolean {
return hasAssignableCause(throwable,
IOException::class.java)
}

/**
* Calls [hasCause] with the `checkSubtypes` parameter set to false.
*/
@JvmStatic
fun hasExactCause(throwable: Throwable, vararg causesToCheck: Class<*>): Boolean {
return hasCause(throwable, false, *causesToCheck)
}

/**
* Calls [hasCause] with the `checkSubtypes` parameter set to true.
*/
@JvmStatic
fun hasAssignableCause(throwable: Throwable?, vararg causesToCheck: Class<*>): Boolean {
return hasCause(throwable, true, *causesToCheck)
}

/**
* Check if throwable has some cause from the causes to check, or is itself in it.
*
* If `checkIfAssignable` is true, not only the exact type will be considered equals, but also its subtypes.
*
* @param throwable throwable that will be checked.
* @param checkSubtypes if subtypes are also checked.
* @param causesToCheck an array of causes to check.
*
* @see Class.isAssignableFrom
*/
@JvmStatic
tailrec fun hasCause(throwable: Throwable?, checkSubtypes: Boolean, vararg causesToCheck: Class<*>): Boolean {
if (throwable == null) {
return false
}

// Check if throwable is a subtype of any of the causes to check
causesToCheck.forEach { causeClass ->
if (checkSubtypes) {
if (causeClass.isAssignableFrom(throwable.javaClass)) {
return true
}
} else {
if (causeClass == throwable.javaClass) {
return true
}
}
}

val currentCause: Throwable? = throwable.cause
// Check if cause is not pointing to the same instance, to avoid infinite loops.
if (throwable !== currentCause) {
return hasCause(currentCause, checkSubtypes, *causesToCheck)
}

return false
}
}
}
85 changes: 1 addition & 84 deletions app/src/main/java/org/schabi/newpipe/util/ExtractorHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,6 @@
import org.schabi.newpipe.report.ErrorActivity;
import org.schabi.newpipe.report.UserAction;

import java.io.IOException;
import java.io.InterruptedIOException;
import java.util.Collections;
import java.util.List;

Expand Down Expand Up @@ -289,7 +287,7 @@ public static void handleGeneralException(final Context context, final int servi
Intent intent = new Intent(context, ReCaptchaActivity.class);
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
context.startActivity(intent);
} else if (exception instanceof IOException) {
} else if (ExceptionUtils.isNetworkRelated(exception)) {
Toast.makeText(context, R.string.network_error, Toast.LENGTH_LONG).show();
} else if (exception instanceof ContentNotAvailableException) {
Toast.makeText(context, R.string.content_not_available, Toast.LENGTH_LONG).show();
Expand All @@ -306,85 +304,4 @@ public static void handleGeneralException(final Context context, final int servi
}
});
}

/**
* Check if throwable have the cause that can be assignable from the causes to check.
*
* @see Class#isAssignableFrom(Class)
* @param throwable the throwable to be checked
* @param causesToCheck the causes to check
* @return whether the exception is an instance of a subclass of one of the causes
* or is caused by an instance of a subclass of one of the causes
*/
public static boolean hasAssignableCauseThrowable(final Throwable throwable,
final Class<?>... causesToCheck) {
// Check if getCause is not the same as cause (the getCause is already the root),
// as it will cause a infinite loop if it is
Throwable cause;
Throwable getCause = throwable;

// Check if throwable is a subclass of any of the filtered classes
final Class throwableClass = throwable.getClass();
for (Class<?> causesEl : causesToCheck) {
if (causesEl.isAssignableFrom(throwableClass)) {
return true;
}
}

// Iteratively checks if the root cause of the throwable is a subclass of the filtered class
while ((cause = throwable.getCause()) != null && getCause != cause) {
getCause = cause;
final Class causeClass = cause.getClass();
for (Class<?> causesEl : causesToCheck) {
if (causesEl.isAssignableFrom(causeClass)) {
return true;
}
}
}
return false;
}

/**
* Check if throwable have the exact cause from one of the causes to check.
*
* @param throwable the throwable to be checked
* @param causesToCheck the causes to check
* @return whether the exception is an instance of one of the causes
* or is caused by an instance of one of the causes
*/
public static boolean hasExactCauseThrowable(final Throwable throwable,
final Class<?>... causesToCheck) {
// Check if getCause is not the same as cause (the getCause is already the root),
// as it will cause a infinite loop if it is
Throwable cause;
Throwable getCause = throwable;

for (Class<?> causesEl : causesToCheck) {
if (throwable.getClass().equals(causesEl)) {
return true;
}
}

while ((cause = throwable.getCause()) != null && getCause != cause) {
getCause = cause;
for (Class<?> causesEl : causesToCheck) {
if (cause.getClass().equals(causesEl)) {
return true;
}
}
}
return false;
}

/**
* Check if throwable have Interrupted* exception as one of its causes.
*
* @param throwable the throwable to be checkes
* @return whether the throwable is caused by an interruption
*/
public static boolean isInterruptedCaused(final Throwable throwable) {
return ExtractorHelper.hasExactCauseThrowable(throwable,
InterruptedIOException.class,
InterruptedException.class);
}
}
Loading