Commit 953e4e89 by Oliver Woodman

Merge pull request #8414 from tidoemanuele:tidoemanuele-improve-non-2xx-message

PiperOrigin-RevId: 350797551
parents a7379ee6 6d4b3645
...@@ -550,19 +550,11 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { ...@@ -550,19 +550,11 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
UrlResponseInfo responseInfo = Assertions.checkNotNull(this.responseInfo); UrlResponseInfo responseInfo = Assertions.checkNotNull(this.responseInfo);
int responseCode = responseInfo.getHttpStatusCode(); int responseCode = responseInfo.getHttpStatusCode();
if (responseCode < 200 || responseCode > 299) { if (responseCode < 200 || responseCode > 299) {
byte[] responseBody = Util.EMPTY_BYTE_ARRAY; byte[] responseBody;
ByteBuffer readBuffer = getOrCreateReadBuffer(); try {
while (!readBuffer.hasRemaining()) { responseBody = readResponseBody();
operation.close(); } catch (HttpDataSourceException e) {
readBuffer.clear(); responseBody = Util.EMPTY_BYTE_ARRAY;
readInternal(readBuffer);
if (finished) {
break;
}
readBuffer.flip();
int existingResponseBodyEnd = responseBody.length;
responseBody = Arrays.copyOf(responseBody, responseBody.length + readBuffer.remaining());
readBuffer.get(responseBody, existingResponseBodyEnd, readBuffer.remaining());
} }
InvalidResponseCodeException exception = InvalidResponseCodeException exception =
...@@ -860,6 +852,29 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource { ...@@ -860,6 +852,29 @@ public class CronetDataSource extends BaseDataSource implements HttpDataSource {
} }
/** /**
* Reads the whole response body.
*
* @return The response body.
* @throws HttpDataSourceException If an error occurs reading from the source.
*/
private byte[] readResponseBody() throws HttpDataSourceException {
byte[] responseBody = Util.EMPTY_BYTE_ARRAY;
ByteBuffer readBuffer = getOrCreateReadBuffer();
while (!finished) {
operation.close();
readBuffer.clear();
readInternal(readBuffer);
readBuffer.flip();
if (readBuffer.remaining() > 0) {
int existingResponseBodyEnd = responseBody.length;
responseBody = Arrays.copyOf(responseBody, responseBody.length + readBuffer.remaining());
readBuffer.get(responseBody, existingResponseBodyEnd, readBuffer.remaining());
}
}
return responseBody;
}
/**
* Reads up to {@code buffer.remaining()} bytes of data from {@code currentUrlRequest} and stores * Reads up to {@code buffer.remaining()} bytes of data from {@code currentUrlRequest} and stores
* them into {@code buffer}. If there is an error and {@code buffer == readBuffer}, then it resets * them into {@code buffer}. If there is an error and {@code buffer == readBuffer}, then it resets
* the current {@code readBuffer} object so that it is not reused in the future. * the current {@code readBuffer} object so that it is not reused in the future.
......
...@@ -41,6 +41,7 @@ import com.google.android.exoplayer2.upstream.DataSpec; ...@@ -41,6 +41,7 @@ import com.google.android.exoplayer2.upstream.DataSpec;
import com.google.android.exoplayer2.upstream.DefaultHttpDataSource; import com.google.android.exoplayer2.upstream.DefaultHttpDataSource;
import com.google.android.exoplayer2.upstream.HttpDataSource; import com.google.android.exoplayer2.upstream.HttpDataSource;
import com.google.android.exoplayer2.upstream.HttpDataSource.HttpDataSourceException; import com.google.android.exoplayer2.upstream.HttpDataSource.HttpDataSourceException;
import com.google.android.exoplayer2.upstream.HttpDataSource.InvalidResponseCodeException;
import com.google.android.exoplayer2.upstream.TransferListener; import com.google.android.exoplayer2.upstream.TransferListener;
import com.google.android.exoplayer2.util.Util; import com.google.android.exoplayer2.util.Util;
import java.io.IOException; import java.io.IOException;
...@@ -380,7 +381,8 @@ public final class CronetDataSourceTest { ...@@ -380,7 +381,8 @@ public final class CronetDataSourceTest {
} }
@Test @Test
public void requestOpenPropagatesFailureResponseBody() throws Exception { public void requestOpen_withNon2xxResponseCode_throwsInvalidResponseCodeExceptionWithBody()
throws Exception {
mockResponseStartSuccess(); mockResponseStartSuccess();
// Use a size larger than CronetDataSource.READ_BUFFER_SIZE_BYTES // Use a size larger than CronetDataSource.READ_BUFFER_SIZE_BYTES
int responseLength = 40 * 1024; int responseLength = 40 * 1024;
...@@ -389,8 +391,8 @@ public final class CronetDataSourceTest { ...@@ -389,8 +391,8 @@ public final class CronetDataSourceTest {
try { try {
dataSourceUnderTest.open(testDataSpec); dataSourceUnderTest.open(testDataSpec);
fail("HttpDataSource.InvalidResponseCodeException expected"); fail("InvalidResponseCodeException expected");
} catch (HttpDataSource.InvalidResponseCodeException e) { } catch (InvalidResponseCodeException e) {
assertThat(e.responseBody).isEqualTo(buildTestDataArray(0, responseLength)); assertThat(e.responseBody).isEqualTo(buildTestDataArray(0, responseLength));
// Check for connection not automatically closed. // Check for connection not automatically closed.
verify(mockUrlRequest, never()).cancel(); verify(mockUrlRequest, never()).cancel();
...@@ -400,6 +402,26 @@ public final class CronetDataSourceTest { ...@@ -400,6 +402,26 @@ public final class CronetDataSourceTest {
} }
@Test @Test
public void
requestOpen_withNon2xxResponseCode_andRequestBodyReadFailure_throwsInvalidResponseCodeExceptionWithoutBody()
throws Exception {
mockResponseStartSuccess();
mockReadFailure();
testUrlResponseInfo = createUrlResponseInfo(/* statusCode= */ 500);
try {
dataSourceUnderTest.open(testDataSpec);
fail("InvalidResponseCodeException expected");
} catch (InvalidResponseCodeException e) {
assertThat(e.responseBody).isEmpty();
// Check for connection not automatically closed.
verify(mockUrlRequest, never()).cancel();
verify(mockTransferListener, never())
.onTransferStart(dataSourceUnderTest, testDataSpec, /* isNetwork= */ true);
}
}
@Test
public void requestOpenValidatesContentTypePredicate() { public void requestOpenValidatesContentTypePredicate() {
mockResponseStartSuccess(); mockResponseStartSuccess();
......
...@@ -308,8 +308,7 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource { ...@@ -308,8 +308,7 @@ public class OkHttpDataSource extends BaseDataSource implements HttpDataSource {
try { try {
errorResponseBody = Util.toByteArray(Assertions.checkNotNull(responseByteStream)); errorResponseBody = Util.toByteArray(Assertions.checkNotNull(responseByteStream));
} catch (IOException e) { } catch (IOException e) {
throw new HttpDataSourceException( errorResponseBody = Util.EMPTY_BYTE_ARRAY;
"Error reading non-2xx response body", e, dataSpec, HttpDataSourceException.TYPE_OPEN);
} }
Map<String, List<String>> headers = response.headers().toMultimap(); Map<String, List<String>> headers = response.headers().toMultimap();
closeConnectionQuietly(); closeConnectionQuietly();
......
...@@ -377,8 +377,7 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou ...@@ -377,8 +377,7 @@ public class DefaultHttpDataSource extends BaseDataSource implements HttpDataSou
errorResponseBody = errorResponseBody =
errorStream != null ? Util.toByteArray(errorStream) : Util.EMPTY_BYTE_ARRAY; errorStream != null ? Util.toByteArray(errorStream) : Util.EMPTY_BYTE_ARRAY;
} catch (IOException e) { } catch (IOException e) {
throw new HttpDataSourceException( errorResponseBody = Util.EMPTY_BYTE_ARRAY;
"Error reading non-2xx response body", e, dataSpec, HttpDataSourceException.TYPE_OPEN);
} }
closeConnectionQuietly(); closeConnectionQuietly();
InvalidResponseCodeException exception = InvalidResponseCodeException exception =
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or sign in to comment