From 8c5703e9313db0dde1b37563a1c8c56e0fae2a40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Patrik=20=C3=85kerfeldt?= Date: Tue, 23 Oct 2018 16:25:40 +0200 Subject: [PATCH 1/2] Provide http status message to InvalidResponseCodeException When response code alone is not enough to distinguish error responses, the http status message is helpful. --- .../exoplayer2/ext/cronet/CronetDataSource.java | 5 +++-- .../exoplayer2/ext/okhttp/OkHttpDataSource.java | 2 +- .../exoplayer2/upstream/DefaultHttpDataSource.java | 4 +++- .../android/exoplayer2/upstream/HttpDataSource.java | 10 +++++++++- .../upstream/DefaultLoadErrorHandlingPolicyTest.java | 6 +++--- 5 files changed, 19 insertions(+), 8 deletions(-) diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index 9525983491..d67d8bb107 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -327,7 +327,7 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { int responseCode = responseInfo.getHttpStatusCode(); if (responseCode < 200 || responseCode > 299) { InvalidResponseCodeException exception = new InvalidResponseCodeException(responseCode, - responseInfo.getAllHeaders(), currentDataSpec); + responseInfo.getHttpStatusText(), responseInfo.getAllHeaders(), currentDataSpec); if (responseCode == 416) { exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); } @@ -611,7 +611,8 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { // The industry standard is to disregard POST redirects when the status code is 307 or 308. if (responseCode == 307 || responseCode == 308) { exception = - new InvalidResponseCodeException(responseCode, info.getAllHeaders(), currentDataSpec); + new InvalidResponseCodeException(responseCode, info.getHttpStatusText(), + info.getAllHeaders(), currentDataSpec); operation.open(); return; } diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java index 2707f539bc..8091c7c94b 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java @@ -173,7 +173,7 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { Map> headers = response.headers().toMultimap(); closeConnectionQuietly(); InvalidResponseCodeException exception = new InvalidResponseCodeException( - responseCode, headers, dataSpec); + responseCode, response.message(), headers, dataSpec); if (responseCode == 416) { exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java index 3673af5540..c6749e6c8f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/DefaultHttpDataSource.java @@ -283,8 +283,10 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou } int responseCode; + String responseMessage; try { responseCode = connection.getResponseCode(); + responseMessage = connection.getResponseMessage(); } catch (IOException e) { closeConnectionQuietly(); throw new HttpDataSourceException("Unable to connect to " + dataSpec.uri.toString(), e, @@ -296,7 +298,7 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou Map> headers = connection.getHeaderFields(); closeConnectionQuietly(); InvalidResponseCodeException exception = - new InvalidResponseCodeException(responseCode, headers, dataSpec); + new InvalidResponseCodeException(responseCode, responseMessage, headers, dataSpec); if (responseCode == 416) { exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java index 0be7b857df..0ee28b260f 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java @@ -16,6 +16,7 @@ package com.google.android.exoplayer2.upstream; import android.support.annotation.IntDef; +import android.support.annotation.Nullable; import android.text.TextUtils; import com.google.android.exoplayer2.util.Predicate; import com.google.android.exoplayer2.util.Util; @@ -294,15 +295,22 @@ public interface HttpDataSource extends DataSource { */ public final int responseCode; + /** + * The HTTP status message. + */ + public final @Nullable String responseMessage; + /** * An unmodifiable map of the response header fields and values. */ public final Map> headerFields; - public InvalidResponseCodeException(int responseCode, Map> headerFields, + public InvalidResponseCodeException(int responseCode, @Nullable String responseMessage, + Map> headerFields, DataSpec dataSpec) { super("Response code: " + responseCode, dataSpec, TYPE_OPEN); this.responseCode = responseCode; + this.responseMessage = responseMessage; this.headerFields = headerFields; } diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java index 0c2b938a64..d656e0dfcc 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java @@ -34,7 +34,7 @@ public final class DefaultLoadErrorHandlingPolicyTest { @Test public void getBlacklistDurationMsFor_blacklist404() { InvalidResponseCodeException exception = - new InvalidResponseCodeException(404, Collections.emptyMap(), new DataSpec(Uri.EMPTY)); + new InvalidResponseCodeException(404, "Not Found", Collections.emptyMap(), new DataSpec(Uri.EMPTY)); assertThat(getDefaultPolicyBlacklistOutputFor(exception)) .isEqualTo(DefaultLoadErrorHandlingPolicy.DEFAULT_TRACK_BLACKLIST_MS); } @@ -42,7 +42,7 @@ public final class DefaultLoadErrorHandlingPolicyTest { @Test public void getBlacklistDurationMsFor_blacklist410() { InvalidResponseCodeException exception = - new InvalidResponseCodeException(410, Collections.emptyMap(), new DataSpec(Uri.EMPTY)); + new InvalidResponseCodeException(410, "Gone", Collections.emptyMap(), new DataSpec(Uri.EMPTY)); assertThat(getDefaultPolicyBlacklistOutputFor(exception)) .isEqualTo(DefaultLoadErrorHandlingPolicy.DEFAULT_TRACK_BLACKLIST_MS); } @@ -50,7 +50,7 @@ public final class DefaultLoadErrorHandlingPolicyTest { @Test public void getBlacklistDurationMsFor_dontBlacklistUnexpectedHttpCodes() { InvalidResponseCodeException exception = - new InvalidResponseCodeException(500, Collections.emptyMap(), new DataSpec(Uri.EMPTY)); + new InvalidResponseCodeException(500, "Internal Server Error", Collections.emptyMap(), new DataSpec(Uri.EMPTY)); assertThat(getDefaultPolicyBlacklistOutputFor(exception)).isEqualTo(C.TIME_UNSET); } From 70ccbc6eb8f82488d7163af606e81ef358acd640 Mon Sep 17 00:00:00 2001 From: tonihei Date: Tue, 23 Oct 2018 18:46:52 +0200 Subject: [PATCH 2/2] Fix review comments See https://github.com/google/ExoPlayer/pull/5004 --- .../exoplayer2/ext/cronet/CronetDataSource.java | 15 +++++++++++---- .../exoplayer2/ext/okhttp/OkHttpDataSource.java | 4 ++-- .../exoplayer2/upstream/HttpDataSource.java | 8 +++++--- .../DefaultLoadErrorHandlingPolicyTest.java | 3 ++- 4 files changed, 20 insertions(+), 10 deletions(-) diff --git a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java index d67d8bb107..9f381d0053 100644 --- a/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java +++ b/extensions/cronet/src/main/java/com/google/android/exoplayer2/ext/cronet/CronetDataSource.java @@ -326,8 +326,12 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { // Check for a valid response code. int responseCode = responseInfo.getHttpStatusCode(); if (responseCode < 200 || responseCode > 299) { - InvalidResponseCodeException exception = new InvalidResponseCodeException(responseCode, - responseInfo.getHttpStatusText(), responseInfo.getAllHeaders(), currentDataSpec); + InvalidResponseCodeException exception = + new InvalidResponseCodeException( + responseCode, + responseInfo.getHttpStatusText(), + responseInfo.getAllHeaders(), + currentDataSpec); if (responseCode == 416) { exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); } @@ -611,8 +615,11 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { // The industry standard is to disregard POST redirects when the status code is 307 or 308. if (responseCode == 307 || responseCode == 308) { exception = - new InvalidResponseCodeException(responseCode, info.getHttpStatusText(), - info.getAllHeaders(), currentDataSpec); + new InvalidResponseCodeException( + responseCode, + info.getHttpStatusText(), + info.getAllHeaders(), + currentDataSpec); operation.open(); return; } diff --git a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java index 8091c7c94b..778277fdbc 100644 --- a/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java +++ b/extensions/okhttp/src/main/java/com/google/android/exoplayer2/ext/okhttp/OkHttpDataSource.java @@ -172,8 +172,8 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { if (!response.isSuccessful()) { Map> headers = response.headers().toMultimap(); closeConnectionQuietly(); - InvalidResponseCodeException exception = new InvalidResponseCodeException( - responseCode, response.message(), headers, dataSpec); + InvalidResponseCodeException exception = + new InvalidResponseCodeException(responseCode, response.message(), headers, dataSpec); if (responseCode == 416) { exception.initCause(new DataSourceException(DataSourceException.POSITION_OUT_OF_RANGE)); } diff --git a/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java b/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java index 0ee28b260f..e73901eccb 100644 --- a/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java +++ b/library/core/src/main/java/com/google/android/exoplayer2/upstream/HttpDataSource.java @@ -298,15 +298,17 @@ public interface HttpDataSource extends DataSource { /** * The HTTP status message. */ - public final @Nullable String responseMessage; + @Nullable public final String responseMessage; /** * An unmodifiable map of the response header fields and values. */ public final Map> headerFields; - public InvalidResponseCodeException(int responseCode, @Nullable String responseMessage, - Map> headerFields, + public InvalidResponseCodeException( + int responseCode, + @Nullable String responseMessage, + Map> headerFields, DataSpec dataSpec) { super("Response code: " + responseCode, dataSpec, TYPE_OPEN); this.responseCode = responseCode; diff --git a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java index d656e0dfcc..f0f737828c 100644 --- a/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java +++ b/library/core/src/test/java/com/google/android/exoplayer2/upstream/DefaultLoadErrorHandlingPolicyTest.java @@ -50,7 +50,8 @@ public final class DefaultLoadErrorHandlingPolicyTest { @Test public void getBlacklistDurationMsFor_dontBlacklistUnexpectedHttpCodes() { InvalidResponseCodeException exception = - new InvalidResponseCodeException(500, "Internal Server Error", Collections.emptyMap(), new DataSpec(Uri.EMPTY)); + new InvalidResponseCodeException( + 500, "Internal Server Error", Collections.emptyMap(), new DataSpec(Uri.EMPTY)); assertThat(getDefaultPolicyBlacklistOutputFor(exception)).isEqualTo(C.TIME_UNSET); }