From 1a92dfb019c55543d8e4e640d5386e87e40a735b Mon Sep 17 00:00:00 2001 From: John Zhen Mo Date: Tue, 20 Feb 2018 22:35:25 -0800 Subject: [PATCH] -Changed global Rx exception handling to no longer trigger error activity if the exception is undeliverable. -Added debug settings to force reporting of undeliverable Rx exceptions. -Changed back MediaSourceManager to use serial disposable for syncing. --- .../java/org/schabi/newpipe/DebugApp.java | 6 ++ app/src/main/java/org/schabi/newpipe/App.java | 55 +++++++++++++++---- .../player/playback/MediaSourceManager.java | 7 ++- app/src/main/res/values/settings_keys.xml | 2 + app/src/main/res/values/strings.xml | 4 ++ app/src/main/res/xml/debug_settings.xml | 5 ++ 6 files changed, 66 insertions(+), 13 deletions(-) diff --git a/app/src/debug/java/org/schabi/newpipe/DebugApp.java b/app/src/debug/java/org/schabi/newpipe/DebugApp.java index 1ba837cdd..df4949edd 100644 --- a/app/src/debug/java/org/schabi/newpipe/DebugApp.java +++ b/app/src/debug/java/org/schabi/newpipe/DebugApp.java @@ -58,6 +58,12 @@ public class DebugApp extends App { Stetho.initialize(initializer); } + @Override + protected boolean isDisposedRxExceptionsReported() { + return PreferenceManager.getDefaultSharedPreferences(this) + .getBoolean(getString(R.string.allow_disposed_exceptions_key), true); + } + @Override protected RefWatcher installLeakCanary() { return LeakCanary.refWatcher(this) diff --git a/app/src/main/java/org/schabi/newpipe/App.java b/app/src/main/java/org/schabi/newpipe/App.java index b15a38aae..9de6f183d 100644 --- a/app/src/main/java/org/schabi/newpipe/App.java +++ b/app/src/main/java/org/schabi/newpipe/App.java @@ -30,9 +30,13 @@ import org.schabi.newpipe.util.StateSaver; import java.io.IOException; import java.io.InterruptedIOException; import java.net.SocketException; +import java.util.Collections; +import java.util.List; import io.reactivex.annotations.NonNull; import io.reactivex.exceptions.CompositeException; +import io.reactivex.exceptions.MissingBackpressureException; +import io.reactivex.exceptions.OnErrorNotImplementedException; import io.reactivex.exceptions.UndeliverableException; import io.reactivex.functions.Consumer; import io.reactivex.plugins.RxJavaPlugins; @@ -99,31 +103,58 @@ public class App extends Application { RxJavaPlugins.setErrorHandler(new Consumer() { @Override public void accept(@NonNull Throwable throwable) throws Exception { - Log.e(TAG, "RxJavaPlugins.ErrorHandler called with -> : throwable = [" + throwable.getClass().getName() + "]"); + Log.e(TAG, "RxJavaPlugins.ErrorHandler called with -> : " + + "throwable = [" + throwable.getClass().getName() + "]"); if (throwable instanceof UndeliverableException) { // As UndeliverableException is a wrapper, get the cause of it to get the "real" exception throwable = throwable.getCause(); } + final List errors; if (throwable instanceof CompositeException) { - for (Throwable element : ((CompositeException) throwable).getExceptions()) { - if (checkThrowable(element)) return; + errors = ((CompositeException) throwable).getExceptions(); + } else { + errors = Collections.singletonList(throwable); + } + + for (final Throwable error : errors) { + if (isThrowableIgnored(error)) return; + if (isThrowableCritical(error)) { + reportException(error); + return; } } - if (checkThrowable(throwable)) return; + // Out-of-lifecycle exceptions should only be reported if a debug user wishes so, + // When exception is not reported, log it + if (isDisposedRxExceptionsReported()) { + reportException(throwable); + } else { + Log.e(TAG, "RxJavaPlugin: Undeliverable Exception received: ", throwable); + } + } + private boolean isThrowableIgnored(@NonNull final Throwable throwable) { + // Don't crash the application over a simple network problem + return ExtractorHelper.hasAssignableCauseThrowable(throwable, + IOException.class, SocketException.class, // network api cancellation + InterruptedException.class, InterruptedIOException.class); // blocking code disposed + } + + private boolean isThrowableCritical(@NonNull final Throwable throwable) { + // Though these exceptions cannot be ignored + return ExtractorHelper.hasAssignableCauseThrowable(throwable, + NullPointerException.class, IllegalArgumentException.class, // bug in app + OnErrorNotImplementedException.class, MissingBackpressureException.class, + IllegalStateException.class); // bug in operator + } + + private void reportException(@NonNull final Throwable throwable) { // Throw uncaught exception that will trigger the report system Thread.currentThread().getUncaughtExceptionHandler() .uncaughtException(Thread.currentThread(), throwable); } - - private boolean checkThrowable(@NonNull Throwable throwable) { - // Don't crash the application over a simple network problem - return ExtractorHelper.hasAssignableCauseThrowable(throwable, - IOException.class, SocketException.class, InterruptedException.class, InterruptedIOException.class); - } }); } @@ -177,4 +208,8 @@ public class App extends Application { protected RefWatcher installLeakCanary() { return RefWatcher.DISABLED; } + + protected boolean isDisposedRxExceptionsReported() { + return true; + } } diff --git a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java index a4438af70..54eb4078a 100644 --- a/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java +++ b/app/src/main/java/org/schabi/newpipe/player/playback/MediaSourceManager.java @@ -22,6 +22,7 @@ import io.reactivex.android.schedulers.AndroidSchedulers; import io.reactivex.annotations.NonNull; import io.reactivex.disposables.CompositeDisposable; import io.reactivex.disposables.Disposable; +import io.reactivex.disposables.SerialDisposable; import io.reactivex.functions.Consumer; import io.reactivex.subjects.PublishSubject; @@ -45,7 +46,7 @@ public class MediaSourceManager { private DynamicConcatenatingMediaSource sources; private Subscription playQueueReactor; - private CompositeDisposable syncReactor; + private SerialDisposable syncReactor; private PlayQueueItem syncedItem; @@ -69,7 +70,7 @@ public class MediaSourceManager { this.windowSize = windowSize; this.loadDebounceMillis = loadDebounceMillis; - this.syncReactor = new CompositeDisposable(); + this.syncReactor = new SerialDisposable(); this.debouncedLoadSignal = PublishSubject.create(); this.debouncedLoader = getDebouncedLoader(); @@ -251,7 +252,7 @@ public class MediaSourceManager { final Disposable sync = currentItem.getStream() .observeOn(AndroidSchedulers.mainThread()) .subscribe(onSuccess, onError); - syncReactor.add(sync); + syncReactor.set(sync); } } diff --git a/app/src/main/res/values/settings_keys.xml b/app/src/main/res/values/settings_keys.xml index f5b6802e8..fc31ee02c 100644 --- a/app/src/main/res/values/settings_keys.xml +++ b/app/src/main/res/values/settings_keys.xml @@ -87,6 +87,8 @@ debug_pref_screen_key allow_heap_dumping_key + allow_disposed_exceptions_key + theme light_theme diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 10f9272af..495842092 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -416,4 +416,8 @@ Enable LeakCanary Memory leak monitoring may cause app to become unresponsive when heap dumping + + Report Out-of-Lifecycle Errors + Force reporting of undeliverable Rx exceptions occurring outside of fragment or activity lifecycle after dispose + diff --git a/app/src/main/res/xml/debug_settings.xml b/app/src/main/res/xml/debug_settings.xml index 9b0fd00d6..67705e018 100644 --- a/app/src/main/res/xml/debug_settings.xml +++ b/app/src/main/res/xml/debug_settings.xml @@ -10,4 +10,9 @@ android:title="@string/enable_leak_canary_title" android:summary="@string/enable_leak_canary_summary"/> +