Skip to content

Commit 52197b2

Browse files
polbinspolbins
authored
fix: use range for common success/failure status codes (#276)
* fix: use range for common success/failure status codes * use statusCode ext function
1 parent a498728 commit 52197b2

File tree

7 files changed

+132
-73
lines changed

7 files changed

+132
-73
lines changed

‎android/src/test/kotlin/com/amplitude/android/ResponseHandlerIntegrationTest.kt

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ class ResponseHandlerIntegrationTest {
114114
amplitude.isBuilt.await()
115115

116116
// verify it will attempt to retry, but single event file will be removed
117-
val code = HttpStatus.PAYLOAD_TOO_LARGE.code
117+
val code = HttpStatus.PAYLOAD_TOO_LARGE.statusCode
118118
val body = "{\"code\": \"$code\", \"error\": \"payload too large\"}"
119119
server.enqueue(MockResponse().setBody(body).setResponseCode(code))
120120
amplitude.track("test event 1", options = options)
@@ -151,12 +151,12 @@ class ResponseHandlerIntegrationTest {
151151
storageProvider = InMemoryStorageProvider()
152152
)
153153
)
154-
val expectedSuccesEvents = 5
154+
val expectedSuccessEvents = 5
155155
server.enqueue(
156156
MockResponse().setBody("{\"code\": \"413\", \"error\": \"payload too large\"}")
157157
.setResponseCode(413)
158158
)
159-
repeat(expectedSuccesEvents) {
159+
repeat(expectedSuccessEvents) {
160160
server.enqueue(MockResponse().setBody("{\"code\": \"200\"}").setResponseCode(200))
161161
}
162162
var eventsProcessed = 0
@@ -205,8 +205,8 @@ class ResponseHandlerIntegrationTest {
205205

206206
// verify we completed processing for the events after file split
207207
assertEquals(4, server.requestCount)
208-
assertEquals(expectedSuccesEvents, statusMap[200])
209-
assertEquals(expectedSuccesEvents, eventsProcessed)
208+
assertEquals(expectedSuccessEvents, statusMap[200])
209+
assertEquals(expectedSuccessEvents, eventsProcessed)
210210
}
211211

212212
@Test
@@ -271,22 +271,22 @@ class ResponseHandlerIntegrationTest {
271271

272272
@Test
273273
fun `test handle bad request response retry`() = runTest {
274-
testRequestFailureAndRetry(HttpStatus.BAD_REQUEST.code)
274+
testRequestFailureAndRetry(HttpStatus.BAD_REQUEST.statusCode)
275275
}
276276

277277
@Test
278278
fun `test handle too many requests response retry`() = runTest {
279-
testRequestFailureAndRetry(HttpStatus.TOO_MANY_REQUESTS.code)
279+
testRequestFailureAndRetry(HttpStatus.TOO_MANY_REQUESTS.statusCode)
280280
}
281281

282282
@Test
283283
fun `test handle timeout response retry`() = runTest {
284-
testRequestFailureAndRetry(HttpStatus.TIMEOUT.code)
284+
testRequestFailureAndRetry(HttpStatus.TIMEOUT.statusCode)
285285
}
286286

287287
@Test
288288
fun `test handle failed response retry`() = runTest {
289-
testRequestFailureAndRetry(HttpStatus.FAILED.code)
289+
testRequestFailureAndRetry(HttpStatus.FAILED.statusCode)
290290
}
291291

292292
private suspend fun TestScope.testRequestFailureAndRetry(code: Int) {

‎core/src/main/java/com/amplitude/core/utilities/FileResponseHandler.kt

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ class FileResponseHandler(
3535
val eventFilePath = events as String
3636
logger?.debug("Handle response, status: ${successResponse.status}")
3737
val eventsList = parseEvents(eventsString, eventFilePath).toEvents()
38-
triggerEventsCallback(eventsList, HttpStatus.SUCCESS.code, "Event sent success.")
38+
triggerEventsCallback(eventsList, HttpStatus.SUCCESS.statusCode, "Event sent success.")
3939
scope.launch(storageDispatcher) {
4040
storage.removeFile(eventFilePath)
4141
}
@@ -52,7 +52,7 @@ class FileResponseHandler(
5252
val eventFilePath = events as String
5353
val eventsList = parseEvents(eventsString, eventFilePath).toEvents()
5454
if (badRequestResponse.isInvalidApiKeyResponse()) {
55-
triggerEventsCallback(eventsList, HttpStatus.BAD_REQUEST.code, badRequestResponse.error)
55+
triggerEventsCallback(eventsList, HttpStatus.BAD_REQUEST.statusCode, badRequestResponse.error)
5656
scope.launch(storageDispatcher) {
5757
storage.removeFile(eventFilePath)
5858
}
@@ -77,7 +77,7 @@ class FileResponseHandler(
7777
return true
7878
}
7979

80-
triggerEventsCallback(eventsToDrop, HttpStatus.BAD_REQUEST.code, badRequestResponse.error)
80+
triggerEventsCallback(eventsToDrop, HttpStatus.BAD_REQUEST.statusCode, badRequestResponse.error)
8181
eventsToRetry.forEach {
8282
eventPipeline.put(it)
8383
}
@@ -104,7 +104,7 @@ class FileResponseHandler(
104104
if (rawEvents.length() == 1) {
105105
val eventsList = rawEvents.toEvents()
106106
triggerEventsCallback(
107-
eventsList, HttpStatus.PAYLOAD_TOO_LARGE.code, payloadTooLargeResponse.error
107+
eventsList, HttpStatus.PAYLOAD_TOO_LARGE.statusCode, payloadTooLargeResponse.error
108108
)
109109
scope.launch(storageDispatcher) {
110110
storage.removeFile(eventFilePath)

‎core/src/main/java/com/amplitude/core/utilities/InMemoryResponseHandler.kt

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ internal class InMemoryResponseHandler(
3333
eventsString: String,
3434
) {
3535
triggerEventsCallback(
36-
events as List<BaseEvent>, HttpStatus.SUCCESS.code, "Event sent success."
36+
events as List<BaseEvent>, HttpStatus.SUCCESS.statusCode, "Event sent success."
3737
)
3838
}
3939

@@ -44,7 +44,7 @@ internal class InMemoryResponseHandler(
4444
): Boolean {
4545
val eventsList = events as List<BaseEvent>
4646
if (badRequestResponse.isInvalidApiKeyResponse()) {
47-
triggerEventsCallback(eventsList, HttpStatus.BAD_REQUEST.code, badRequestResponse.error)
47+
triggerEventsCallback(eventsList, HttpStatus.BAD_REQUEST.statusCode, badRequestResponse.error)
4848
return false
4949
}
5050
val droppedIndices = badRequestResponse.getEventIndicesToDrop()
@@ -57,7 +57,7 @@ internal class InMemoryResponseHandler(
5757
eventsToRetry.add(event)
5858
}
5959
}
60-
triggerEventsCallback(eventsToDrop, HttpStatus.BAD_REQUEST.code, badRequestResponse.error)
60+
triggerEventsCallback(eventsToDrop, HttpStatus.BAD_REQUEST.statusCode, badRequestResponse.error)
6161
eventsToRetry.forEach {
6262
eventPipeline.put(it)
6363
}
@@ -72,7 +72,7 @@ internal class InMemoryResponseHandler(
7272
val eventsList = events as List<BaseEvent>
7373
if (eventsList.size == 1) {
7474
triggerEventsCallback(
75-
eventsList, HttpStatus.PAYLOAD_TOO_LARGE.code, payloadTooLargeResponse.error
75+
eventsList, HttpStatus.PAYLOAD_TOO_LARGE.statusCode, payloadTooLargeResponse.error
7676
)
7777
return
7878
}
@@ -105,7 +105,7 @@ internal class InMemoryResponseHandler(
105105
}
106106
}
107107
triggerEventsCallback(
108-
eventsToDrop, HttpStatus.TOO_MANY_REQUESTS.code, tooManyRequestsResponse.error
108+
eventsToDrop, HttpStatus.TOO_MANY_REQUESTS.statusCode, tooManyRequestsResponse.error
109109
)
110110
eventsToRetryNow.forEach {
111111
eventPipeline.put(it)
@@ -147,7 +147,7 @@ internal class InMemoryResponseHandler(
147147
eventsToRetry.add(event)
148148
}
149149
}
150-
triggerEventsCallback(eventsToDrop, HttpStatus.FAILED.code, failedResponse.error)
150+
triggerEventsCallback(eventsToDrop, HttpStatus.FAILED.statusCode, failedResponse.error)
151151
eventsToRetry.forEach {
152152
eventPipeline.put(it)
153153
}

‎core/src/main/java/com/amplitude/core/utilities/http/AnalyticsResponse.kt

Lines changed: 37 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -3,58 +3,47 @@ package com.amplitude.core.utilities.http
33
import com.amplitude.core.events.BaseEvent
44
import com.amplitude.core.utilities.collectIndices
55
import com.amplitude.core.utilities.getStringWithDefault
6+
import com.amplitude.core.utilities.http.HttpStatus.BAD_REQUEST
7+
import com.amplitude.core.utilities.http.HttpStatus.PAYLOAD_TOO_LARGE
8+
import com.amplitude.core.utilities.http.HttpStatus.SUCCESS
9+
import com.amplitude.core.utilities.http.HttpStatus.TIMEOUT
10+
import com.amplitude.core.utilities.http.HttpStatus.TOO_MANY_REQUESTS
611
import com.amplitude.core.utilities.toIntArray
712
import org.json.JSONObject
813

9-
internal object HttpResponse {
10-
fun createHttpResponse(
11-
code: Int,
12-
responseBody: String?,
13-
): AnalyticsResponse {
14-
return when (code) {
15-
HttpStatus.SUCCESS.code -> SuccessResponse()
16-
17-
HttpStatus.BAD_REQUEST.code -> BadRequestResponse(JSONObject(responseBody))
18-
19-
HttpStatus.PAYLOAD_TOO_LARGE.code -> PayloadTooLargeResponse(JSONObject(responseBody))
20-
21-
HttpStatus.TOO_MANY_REQUESTS.code -> TooManyRequestsResponse(JSONObject(responseBody))
22-
23-
HttpStatus.TIMEOUT.code -> TimeoutResponse()
24-
25-
else -> FailedResponse(parseResponseBodyOrGetDefault(responseBody))
26-
}
27-
}
28-
29-
private fun parseResponseBodyOrGetDefault(responseBody: String?): JSONObject {
30-
val defaultObject = JSONObject()
31-
if (responseBody.isNullOrEmpty()) {
32-
return defaultObject
33-
}
34-
35-
return try {
36-
JSONObject(responseBody)
37-
} catch (ignored: Exception) {
38-
defaultObject.put("error", responseBody)
39-
defaultObject
40-
}
41-
}
42-
}
43-
4414
sealed class AnalyticsResponse(val status: HttpStatus) {
4515
companion object {
4616
fun create(
4717
responseCode: Int,
4818
responseBody: String?,
4919
): AnalyticsResponse {
50-
return HttpResponse.createHttpResponse(responseCode, responseBody)
20+
return when (responseCode) {
21+
in SUCCESS.range -> SuccessResponse()
22+
in BAD_REQUEST.range -> BadRequestResponse(JSONObject(responseBody))
23+
in PAYLOAD_TOO_LARGE.range -> PayloadTooLargeResponse(JSONObject(responseBody))
24+
in TOO_MANY_REQUESTS.range -> TooManyRequestsResponse(JSONObject(responseBody))
25+
in TIMEOUT.range -> TimeoutResponse()
26+
else -> FailedResponse(parseResponseBodyOrGetDefault(responseBody))
27+
}
28+
}
29+
30+
private fun parseResponseBodyOrGetDefault(responseBody: String?): JSONObject {
31+
val defaultObject = JSONObject()
32+
if (responseBody.isNullOrEmpty()) return defaultObject
33+
34+
return try {
35+
JSONObject(responseBody)
36+
} catch (ignored: Exception) {
37+
defaultObject.put("error", responseBody)
38+
defaultObject
39+
}
5140
}
5241
}
5342
}
5443

55-
class SuccessResponse : AnalyticsResponse(HttpStatus.SUCCESS)
44+
class SuccessResponse : AnalyticsResponse(SUCCESS)
5645

57-
class BadRequestResponse(response: JSONObject) : AnalyticsResponse(HttpStatus.BAD_REQUEST) {
46+
class BadRequestResponse(response: JSONObject) : AnalyticsResponse(BAD_REQUEST) {
5847
val error: String = response.getStringWithDefault("error", "")
5948
private var eventsWithInvalidFields: Set<Int> = setOf()
6049
private var eventsWithMissingFields: Set<Int> = setOf()
@@ -102,12 +91,12 @@ class BadRequestResponse(response: JSONObject) : AnalyticsResponse(HttpStatus.BA
10291
}
10392

10493
class PayloadTooLargeResponse(response: JSONObject) :
105-
AnalyticsResponse(HttpStatus.PAYLOAD_TOO_LARGE) {
94+
AnalyticsResponse(PAYLOAD_TOO_LARGE) {
10695
val error: String = response.getStringWithDefault("error", "")
10796
}
10897

10998
class TooManyRequestsResponse(response: JSONObject) :
110-
AnalyticsResponse(HttpStatus.TOO_MANY_REQUESTS) {
99+
AnalyticsResponse(TOO_MANY_REQUESTS) {
111100
private var exceededDailyQuotaUsers: Set<String> = setOf()
112101
private var exceededDailyQuotaDevices: Set<String> = setOf()
113102
private var throttledDevices: Set<String> = setOf()
@@ -141,7 +130,7 @@ class TooManyRequestsResponse(response: JSONObject) :
141130
}
142131
}
143132

144-
class TimeoutResponse : AnalyticsResponse(HttpStatus.TIMEOUT)
133+
class TimeoutResponse : AnalyticsResponse(TIMEOUT)
145134

146135
class FailedResponse(response: JSONObject) : AnalyticsResponse(HttpStatus.FAILED) {
147136
val error: String = response.getStringWithDefault("error", "")
@@ -268,12 +257,16 @@ interface ResponseHandler {
268257
* A request requires a retry if the event file/s are still present and we want to attempt to upload them again.
269258
*/
270259
enum class HttpStatus(
271-
val code: Int
260+
code: Int,
261+
val range: IntRange = code..code,
272262
) {
273-
SUCCESS(200),
263+
SUCCESS(200, range = 200..299),
274264
BAD_REQUEST(400),
275265
TIMEOUT(408),
276266
PAYLOAD_TOO_LARGE(413),
277267
TOO_MANY_REQUESTS(429),
278-
FAILED(500),
268+
FAILED(500, 500..599);
269+
270+
val statusCode: Int
271+
get() = range.first
279272
}

‎core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import org.junit.jupiter.api.AfterEach
1414
import org.junit.jupiter.api.Assertions.assertEquals
1515
import org.junit.jupiter.api.Assertions.assertNotNull
1616
import org.junit.jupiter.api.Assertions.assertNull
17+
import org.junit.jupiter.api.Assertions.assertTrue
1718
import org.junit.jupiter.api.BeforeEach
1819
import org.junit.jupiter.api.Test
1920
import org.junit.jupiter.api.TestInstance
@@ -22,7 +23,7 @@ import java.util.concurrent.TimeUnit
2223
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
2324
class HttpClientTest {
2425
private lateinit var server: MockWebServer
25-
val apiKey = "API_KEY"
26+
private val apiKey = "API_KEY"
2627

2728
@ExperimentalCoroutinesApi
2829
@BeforeEach
@@ -49,7 +50,7 @@ class HttpClientTest {
4950
event.eventType = "test"
5051

5152
val httpClient = spyk(HttpClient(config))
52-
val response = httpClient.upload(JSONUtil.eventsToString(listOf(event)))
53+
httpClient.upload(JSONUtil.eventsToString(listOf(event)))
5354

5455
val request = runRequest()
5556
val result = JSONObject(request?.body?.readUtf8())
@@ -73,7 +74,7 @@ class HttpClientTest {
7374

7475
val httpClient = spyk(HttpClient(config))
7576

76-
val response = httpClient.upload(JSONUtil.eventsToString(listOf(event)))
77+
httpClient.upload(JSONUtil.eventsToString(listOf(event)))
7778

7879
val request = runRequest()
7980
val result = JSONObject(request?.body?.readUtf8())
@@ -100,7 +101,7 @@ class HttpClientTest {
100101
diagnostics.addErrorLog("error")
101102
diagnostics.addMalformedEvent("malformed-event")
102103

103-
val response = httpClient.upload(JSONUtil.eventsToString(listOf(event)), diagnostics.extractDiagnostics())
104+
httpClient.upload(JSONUtil.eventsToString(listOf(event)), diagnostics.extractDiagnostics())
104105

105106
val request = runRequest()
106107
val result = JSONObject(request?.body?.readUtf8())
@@ -133,10 +134,10 @@ class HttpClientTest {
133134
val response = httpClient.upload(JSONUtil.eventsToString(listOf(event)), diagnostics.extractDiagnostics())
134135

135136
runRequest()
136-
assertEquals(200, response.status.code)
137+
assertTrue(200 in response.status.range)
137138

138139
runRequest()
139-
assertEquals(200, response.status.code)
140+
assertTrue(200 in response.status.range)
140141
}
141142

142143
@Test
@@ -156,8 +157,7 @@ class HttpClientTest {
156157
val response = httpClient.upload(JSONUtil.eventsToString(listOf(event)))
157158

158159
runRequest()
159-
// Error code 503 is converted to a 500 in the http client
160-
assertEquals(500, response.status.code)
160+
assertTrue(503 in response.status.range)
161161
val responseBody = response as FailedResponse
162162
assertEquals("<html>Error occurred</html>", responseBody.error)
163163
}

0 commit comments

Comments
 (0)