Skip to content

Commit 325430d

Browse files
authored
Merge pull request #537 from yoavst/bug-fix-error-handling
Fix calling to getInputStream on errors
2 parents 29d75f3 + ab5add7 commit 325430d

File tree

4 files changed

+44
-3
lines changed

4 files changed

+44
-3
lines changed

analytics/src/main/java/com/segment/analytics/Analytics.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import static com.segment.analytics.internal.Utils.assertNotNull;
2828
import static com.segment.analytics.internal.Utils.buffer;
2929
import static com.segment.analytics.internal.Utils.closeQuietly;
30+
import static com.segment.analytics.internal.Utils.getInputStream;
3031
import static com.segment.analytics.internal.Utils.getResourceString;
3132
import static com.segment.analytics.internal.Utils.getSegmentSharedPreferences;
3233
import static com.segment.analytics.internal.Utils.hasPermission;
@@ -360,7 +361,7 @@ void trackAttributionInformation() {
360361

361362
// Read the response body.
362363
Map<String, Object> map =
363-
cartographer.fromJson(buffer(connection.connection.getInputStream()));
364+
cartographer.fromJson(buffer(getInputStream(connection.connection)));
364365
Properties properties = new Properties(map);
365366

366367
track("Install Attributed", properties);

analytics/src/main/java/com/segment/analytics/Client.java

+3-2
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
package com.segment.analytics;
2626

2727
import static com.segment.analytics.internal.Utils.readFully;
28+
import static com.segment.analytics.internal.Utils.getInputStream;
2829
import static java.net.HttpURLConnection.HTTP_OK;
2930

3031
import android.text.TextUtils;
@@ -58,7 +59,7 @@ public void close() throws IOException {
5859
if (responseCode >= 300) {
5960
String responseBody;
6061
try {
61-
responseBody = readFully(connection.getInputStream());
62+
responseBody = readFully(getInputStream(connection));
6263
} catch (IOException e) {
6364
responseBody = "Could not read response body for rejected message: " + e.toString();
6465
}
@@ -73,7 +74,7 @@ public void close() throws IOException {
7374
}
7475

7576
private static Connection createGetConnection(HttpURLConnection connection) throws IOException {
76-
return new Connection(connection, connection.getInputStream(), null) {
77+
return new Connection(connection, getInputStream(connection), null) {
7778
@Override
7879
public void close() throws IOException {
7980
super.close();

analytics/src/main/java/com/segment/analytics/internal/Utils.java

+9
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@
5252
import java.io.InputStream;
5353
import java.io.InputStreamReader;
5454
import java.lang.reflect.Array;
55+
import java.net.HttpURLConnection;
5556
import java.text.ParseException;
5657
import java.util.ArrayList;
5758
import java.util.Collection;
@@ -351,6 +352,14 @@ public static String readFully(BufferedReader reader) throws IOException {
351352
return sb.toString();
352353
}
353354

355+
public static InputStream getInputStream(HttpURLConnection connection) throws IOException {
356+
try {
357+
return connection.getInputStream();
358+
} catch (IOException ignored) {
359+
return connection.getErrorStream();
360+
}
361+
}
362+
354363
/**
355364
* Transforms the given map by replacing the keys mapped by {@code mapper}. Any keys not in the
356365
* mapper preserve their original keys. If a key in the mapper maps to null or a blank string,

analytics/src/test/java/com/segment/analytics/ClientTest.java

+30
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
import com.squareup.okhttp.mockwebserver.MockResponse;
1313
import com.squareup.okhttp.mockwebserver.RecordedRequest;
1414
import com.squareup.okhttp.mockwebserver.rule.MockWebServerRule;
15+
16+
import java.io.FileNotFoundException;
1517
import java.io.IOException;
1618
import java.io.InputStream;
1719
import java.io.OutputStream;
@@ -150,6 +152,34 @@ public void uploadFailureClosesStreamsAndThrowsException() throws Exception {
150152
verify(os).close();
151153
}
152154

155+
@Test
156+
public void uploadFailureWithErrorStreamClosesStreamsAndThrowsException() throws Exception {
157+
OutputStream os = mock(OutputStream.class);
158+
InputStream is = mock(InputStream.class);
159+
when(mockConnection.getOutputStream()).thenReturn(os);
160+
when(mockConnection.getResponseCode()).thenReturn(404);
161+
when(mockConnection.getResponseMessage()).thenReturn("bar");
162+
when(mockConnection.getInputStream()).thenThrow(new FileNotFoundException());
163+
when(mockConnection.getErrorStream()).thenReturn(is);
164+
165+
Client.Connection connection = mockClient.upload();
166+
verify(mockConnection).setDoOutput(true);
167+
verify(mockConnection).setChunkedStreamingMode(0);
168+
169+
try {
170+
connection.close();
171+
fail(">= 300 return code should throw an exception");
172+
} catch (Client.HTTPException e) {
173+
assertThat(e)
174+
.hasMessage(
175+
"HTTP 404: bar. "
176+
+ "Response: Could not read response body for rejected message: "
177+
+ "java.io.IOException: Underlying input stream returned zero bytes");
178+
}
179+
verify(mockConnection).disconnect();
180+
verify(os).close();
181+
}
182+
153183
@Test
154184
public void fetchSettings() throws Exception {
155185
server.enqueue(new MockResponse());

0 commit comments

Comments
 (0)