From e4c9078a0ca8a7e1bfad26db8023d1514d0823ce Mon Sep 17 00:00:00 2001 From: claincly Date: Tue, 20 Jul 2021 16:15:52 +0100 Subject: [PATCH] Infer error code in network-based DataSourceException. In some DataSources, it is not easy to assign an error code at the throw site. For example, CronetDataSource.readInternal() throws SocketTimeoutException on L1033, and is caught at L754 as IOException and is thrown. We need the logic to assign error code for the actual type of the error cause. While we can certainly do in individual DataSources, IMO there's value in making this logic generic at a higher level (like what is in this CL). The catch and translation logic is borrowed from EPII:L646. PiperOrigin-RevId: 385789629 --- .../upstream/DataSourceException.java | 26 ++++++- .../exoplayer2/upstream/HttpDataSource.java | 15 +++-- .../upstream/DataSourceExceptionTest.java | 67 +++++++++++++++++++ .../exoplayer2/ExoPlayerImplInternal.java | 28 +------- 4 files changed, 105 insertions(+), 31 deletions(-) diff --git a/library/common/src/main/java/com/google/android/exoplayer2/upstream/DataSourceException.java b/library/common/src/main/java/com/google/android/exoplayer2/upstream/DataSourceException.java index bd4f9fba22..3d667acd9d 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/upstream/DataSourceException.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/upstream/DataSourceException.java @@ -18,6 +18,8 @@ package com.google.android.exoplayer2.upstream; import androidx.annotation.Nullable; import com.google.android.exoplayer2.PlaybackException; import java.io.IOException; +import java.net.SocketTimeoutException; +import java.net.UnknownHostException; /** Used to specify reason of a DataSource error. */ public class DataSourceException extends IOException { @@ -78,7 +80,7 @@ public class DataSourceException extends IOException { public DataSourceException( String message, Throwable cause, @PlaybackException.ErrorCode int reason) { super(message, cause); - this.reason = reason; + this.reason = inferErrorCode(reason, cause); } /** @@ -90,7 +92,7 @@ public class DataSourceException extends IOException { */ public DataSourceException(Throwable cause, @PlaybackException.ErrorCode int reason) { super(cause); - this.reason = reason; + this.reason = inferErrorCode(reason, cause); } /** @@ -104,4 +106,24 @@ public class DataSourceException extends IOException { super(message); this.reason = reason; } + + @PlaybackException.ErrorCode + private static int inferErrorCode( + @PlaybackException.ErrorCode int reason, @Nullable Throwable cause) { + if (reason != PlaybackException.ERROR_CODE_IO_UNSPECIFIED) { + return reason; + } + + while (cause != null) { + if (cause instanceof UnknownHostException) { + return PlaybackException.ERROR_CODE_IO_DNS_FAILED; + } else if (cause instanceof SocketTimeoutException) { + return PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_TIMEOUT; + } else if (cause instanceof DataSourceException) { + return ((DataSourceException) cause).reason; + } + cause = cause.getCause(); + } + return PlaybackException.ERROR_CODE_IO_UNSPECIFIED; + } } diff --git a/library/common/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java b/library/common/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java index 57a17e022a..42106cb78d 100644 --- a/library/common/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java +++ b/library/common/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java @@ -231,7 +231,7 @@ public interface HttpDataSource extends DataSource { */ public HttpDataSourceException( DataSpec dataSpec, @PlaybackException.ErrorCode int errorCode, @Type int type) { - super(errorCode); + super(assignErrorCode(errorCode, type)); this.dataSpec = dataSpec; this.type = type; } @@ -260,7 +260,7 @@ public interface HttpDataSource extends DataSource { DataSpec dataSpec, @PlaybackException.ErrorCode int errorCode, @Type int type) { - super(message, errorCode); + super(message, assignErrorCode(errorCode, type)); this.dataSpec = dataSpec; this.type = type; } @@ -289,7 +289,7 @@ public interface HttpDataSource extends DataSource { DataSpec dataSpec, @PlaybackException.ErrorCode int errorCode, @Type int type) { - super(cause, errorCode); + super(cause, assignErrorCode(errorCode, type)); this.dataSpec = dataSpec; this.type = type; } @@ -321,10 +321,17 @@ public interface HttpDataSource extends DataSource { DataSpec dataSpec, @PlaybackException.ErrorCode int errorCode, @Type int type) { - super(message, cause, errorCode); + super(message, cause, assignErrorCode(errorCode, type)); this.dataSpec = dataSpec; this.type = type; } + + @PlaybackException.ErrorCode + private static int assignErrorCode(@PlaybackException.ErrorCode int errorCode, @Type int type) { + return errorCode == PlaybackException.ERROR_CODE_IO_UNSPECIFIED && type == TYPE_OPEN + ? PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_FAILED + : errorCode; + } } /** diff --git a/library/common/src/test/java/com/google/android/exoplayer2/upstream/DataSourceExceptionTest.java b/library/common/src/test/java/com/google/android/exoplayer2/upstream/DataSourceExceptionTest.java index 432333f09f..1cda53516b 100644 --- a/library/common/src/test/java/com/google/android/exoplayer2/upstream/DataSourceExceptionTest.java +++ b/library/common/src/test/java/com/google/android/exoplayer2/upstream/DataSourceExceptionTest.java @@ -17,9 +17,12 @@ package com.google.android.exoplayer2.upstream; import static com.google.common.truth.Truth.assertThat; +import android.net.Uri; import androidx.test.ext.junit.runners.AndroidJUnit4; import com.google.android.exoplayer2.PlaybackException; import java.io.IOException; +import java.net.SocketTimeoutException; +import java.net.UnknownHostException; import org.junit.Test; import org.junit.runner.RunWith; @@ -55,4 +58,68 @@ public class DataSourceExceptionTest { IOException e = new IOException(new IOException(cause)); assertThat(DataSourceException.isCausedByPositionOutOfRange(e)).isFalse(); } + + @Test + public void constructor_withNestedCausesAndUnspecifiedErrorCodes_assignsCorrectErrorCodes() { + DataSourceException exception = + new DataSourceException( + new UnknownHostException(), PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason).isEqualTo(PlaybackException.ERROR_CODE_IO_DNS_FAILED); + + exception = + new DataSourceException( + new SocketTimeoutException(), PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason) + .isEqualTo(PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_TIMEOUT); + + exception = + new DataSourceException( + new IOException(new SocketTimeoutException()), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason) + .isEqualTo(PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_TIMEOUT); + + exception = + new DataSourceException( + new DataSourceException( + new SocketTimeoutException(), PlaybackException.ERROR_CODE_IO_UNSPECIFIED), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason) + .isEqualTo(PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_TIMEOUT); + + exception = + new DataSourceException( + new DataSourceException( + new DataSourceException(PlaybackException.ERROR_CODE_IO_READ_POSITION_OUT_OF_RANGE), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason) + .isEqualTo(PlaybackException.ERROR_CODE_IO_READ_POSITION_OUT_OF_RANGE); + + exception = + new DataSourceException( + new HttpDataSource.CleartextNotPermittedException( + new IOException(), new DataSpec(Uri.parse("test"))), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason).isEqualTo(PlaybackException.ERROR_CODE_IO_CLEARTEXT_NOT_PERMITTED); + + exception = + new DataSourceException( + new HttpDataSource.HttpDataSourceException( + new IOException(), + new DataSpec(Uri.parse("test")), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED, + HttpDataSource.HttpDataSourceException.TYPE_OPEN), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason) + .isEqualTo(PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_FAILED); + + exception = + new DataSourceException( + new DataSourceException( + new DataSourceException(PlaybackException.ERROR_CODE_IO_UNSPECIFIED), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED), + PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + assertThat(exception.reason).isEqualTo(PlaybackException.ERROR_CODE_IO_UNSPECIFIED); + } } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java index d57ccccc81..0975378671 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/ExoPlayerImplInternal.java @@ -48,9 +48,8 @@ import com.google.android.exoplayer2.trackselection.ExoTrackSelection; import com.google.android.exoplayer2.trackselection.TrackSelector; import com.google.android.exoplayer2.trackselection.TrackSelectorResult; import com.google.android.exoplayer2.upstream.BandwidthMeter; +import com.google.android.exoplayer2.upstream.DataSourceException; import com.google.android.exoplayer2.upstream.FileDataSource; -import com.google.android.exoplayer2.upstream.HttpDataSource; -import com.google.android.exoplayer2.upstream.UdpDataSource; import com.google.android.exoplayer2.util.Assertions; import com.google.android.exoplayer2.util.Clock; import com.google.android.exoplayer2.util.HandlerWrapper; @@ -60,8 +59,6 @@ import com.google.android.exoplayer2.util.Util; import com.google.common.base.Supplier; import com.google.common.collect.ImmutableList; import java.io.IOException; -import java.net.SocketTimeoutException; -import java.net.UnknownHostException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -598,27 +595,8 @@ import java.util.concurrent.atomic.AtomicBoolean; errorCode = PlaybackException.ERROR_CODE_UNSPECIFIED; } handleIoException(e, errorCode); - } catch (HttpDataSource.CleartextNotPermittedException e) { - handleIoException(e, PlaybackException.ERROR_CODE_IO_CLEARTEXT_NOT_PERMITTED); - } catch (HttpDataSource.InvalidResponseCodeException e) { - handleIoException(e, PlaybackException.ERROR_CODE_IO_BAD_HTTP_STATUS); - } catch (HttpDataSource.HttpDataSourceException | UdpDataSource.UdpDataSourceException e) { - @ErrorCode int errorCode; - @Nullable Throwable cause = e.getCause(); - if (cause instanceof UnknownHostException) { - errorCode = PlaybackException.ERROR_CODE_IO_DNS_FAILED; - } else if (cause instanceof SocketTimeoutException) { - errorCode = PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_TIMEOUT; - } else if (e instanceof HttpDataSource.HttpDataSourceException) { - int type = ((HttpDataSource.HttpDataSourceException) e).type; - errorCode = - type == HttpDataSource.HttpDataSourceException.TYPE_OPEN - ? PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_FAILED - : PlaybackException.ERROR_CODE_IO_NETWORK_CONNECTION_CLOSED; - } else { - errorCode = PlaybackException.ERROR_CODE_IO_UNSPECIFIED; - } - handleIoException(e, errorCode); + } catch (DataSourceException e) { + handleIoException(e, e.reason); } catch (BehindLiveWindowException e) { handleIoException(e, PlaybackException.ERROR_CODE_BEHIND_LIVE_WINDOW); } catch (IOException e) {