Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: reduce batch size if API returns 413 #130

Merged
merged 5 commits into from
May 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
## Next

## 3.2.1 - 2024-05-08

- fix: reduce batch size if API returns 413 ([#130](https://github.com/PostHog/posthog-android/pull/130))
- `screenshot` image is now a JPEG at 30% quality to reduce size

## 3.2.0 - 2024-04-30

- recording: add `screenshot` option for session replay instead of wireframe ([#127](https://github.com/PostHog/posthog-android/pull/127))
Expand Down
2 changes: 1 addition & 1 deletion posthog-android/lint-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

<issue
id="GradleDependency"
message="A newer version of androidx.core:core than 1.5.0 is available: 1.13.0"
message="A newer version of androidx.core:core than 1.5.0 is available: 1.13.1"
errorLine1=" implementation(&quot;androidx.core:core:${PosthogBuildConfig.Dependencies.ANDROIDX_CORE}&quot;)"
errorLine2=" ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
<location
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -805,9 +805,11 @@ public class PostHogReplayIntegration(
}

ByteArrayOutputStream(allocationByteCount).use {
compress(Bitmap.CompressFormat.PNG, 30, it)
// we can make format and type configurable
compress(Bitmap.CompressFormat.JPEG, 30, it)
val byteArray = it.toByteArray()
return Base64.encodeToString(byteArray, Base64.DEFAULT)
val encoded = Base64.encodeToString(byteArray, Base64.DEFAULT) ?: return null
return "data:image/jpeg;base64,$encoded"
}
}

Expand Down
29 changes: 26 additions & 3 deletions posthog/src/main/java/com/posthog/internal/PostHogQueue.kt
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import java.util.UUID
import java.util.concurrent.ExecutorService
import java.util.concurrent.atomic.AtomicBoolean
import kotlin.concurrent.schedule
import kotlin.math.max
import kotlin.math.min

/**
Expand Down Expand Up @@ -189,9 +190,8 @@ internal class PostHogQueue(
}
}
} catch (e: PostHogApiError) {
if (e.statusCode < 400) {
deleteFiles = false
}
deleteFiles = deleteFilesIfAPIError(e, config)

throw e
} catch (e: IOException) {
// no connection should try again
Expand Down Expand Up @@ -315,3 +315,26 @@ internal class PostHogQueue(
return tempFiles
}
}

private fun calcFloor(currentValue: Int): Int {
return max(currentValue.floorDiv(2), 1)
}

internal fun deleteFilesIfAPIError(
e: PostHogApiError,
config: PostHogConfig,
): Boolean {
if (e.statusCode < 400) {
return false
}
// workaround due to png images exceed our max. limit in kafka
if (e.statusCode == 413 && config.maxBatchSize > 1) {
// try to reduce the batch size and flushAt until its 1
// and if it still throws 413 in the next retry, delete the files since we cannot handle anyway
config.maxBatchSize = calcFloor(config.maxBatchSize)
config.flushAt = calcFloor(config.flushAt)
Comment on lines +334 to +335
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice... it makes me wonder how we'd know it was happening 🤔 but I guess we can figure out where we're returning 413 in capture and send an ingestion warning

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah we probably have 413s in our logging systems, adding one more custom event to the batch is likely to increase the payload :(


return false
}
return true
}
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,8 @@ internal class PostHogSendCachedEventsIntegration(
PostHogApiEndpoint.SNAPSHOT -> api.snapshot(events)
}
} catch (e: PostHogApiError) {
if (e.statusCode < 400) {
deleteFiles = false
}
deleteFiles = deleteFilesIfAPIError(e, config)

throw e
} catch (e: IOException) {
// no connection should try again
Expand Down
33 changes: 33 additions & 0 deletions posthog/src/test/java/com/posthog/internal/PostHogQueueTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import com.posthog.mockHttp
import com.posthog.shutdownAndAwaitTermination
import okhttp3.mockwebserver.MockResponse
import okhttp3.mockwebserver.SocketPolicy
import org.junit.Assert.assertFalse
import org.junit.Rule
import org.junit.rules.TemporaryFolder
import java.io.File
Expand Down Expand Up @@ -330,4 +331,36 @@ internal class PostHogQueueTest {
assertEquals(0, File(path, API_KEY).listFiles()!!.size)
assertEquals(4, http.requestCount)
}

@Test
fun `reduces batch size if 413`() {
val e = PostHogApiError(413, "", null)
val config = PostHogConfig(API_KEY)

assertFalse(deleteFilesIfAPIError(e, config))
assertEquals(config.maxBatchSize, 25) // default 50
assertEquals(config.flushAt, 10) // default 20
}

@Test
fun `delete files if batch is min already`() {
val e = PostHogApiError(413, "", null)
val config =
PostHogConfig(API_KEY).apply {
maxBatchSize = 1
flushAt = 1
}

assertTrue(deleteFilesIfAPIError(e, config))
assertEquals(config.maxBatchSize, 1)
assertEquals(config.flushAt, 1)
}

@Test
fun `delete files if errored`() {
val e = PostHogApiError(400, "", null)
val config = PostHogConfig(API_KEY)

assertTrue(deleteFilesIfAPIError(e, config))
}
}
Loading