From 7ed38e55b4b1c7eac006f129133032336575db19 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 10 Mar 2025 11:15:37 +0100 Subject: [PATCH 1/3] chore: delete queue files if serialization fails --- CHANGELOG.md | 1 + .../java/com/posthog/internal/PostHogQueue.kt | 30 ++++++++++++++----- .../PostHogSendCachedEventsIntegration.kt | 29 ++++++++++++++---- 3 files changed, 48 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77d54d3f..8c86f58d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ ## Next - feat: support reuse of `anonymousId` between user changes ([#229](https://github.com/PostHog/posthog-android/pull/229)) +- chore: delete queue files if serialization fail ## 3.11.3 - 2025-02-26 diff --git a/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt b/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt index 71f8ccce..df47fdbc 100644 --- a/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt +++ b/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt @@ -81,16 +81,23 @@ internal class PostHogQueue( deque.add(file) } + var hasPendingEvents = true try { val os = config.encryption?.encrypt(file.outputStream()) ?: file.outputStream() os.use { theOutputStream -> config.serializer.serialize(event, theOutputStream.writer().buffered()) } config.logger.log("Queued Event ${event.event}: ${file.name}.") - - flushIfOverThreshold() } catch (e: Throwable) { + hasPendingEvents = false config.logger.log("Event ${event.event}: ${file.name} failed to parse: $e.") + + // if for some reason the file failed to serialize, lets delete it + file.deleteSafely(config) + } + + if (hasPendingEvents) { + flushIfOverThreshold() } } } @@ -163,6 +170,17 @@ internal class PostHogQueue( } } + private fun deleteFileSafely( + file: File, + throwable: Throwable? = null, + ) { + synchronized(dequeLock) { + deque.remove(file) + } + file.deleteSafely(config) + config.logger.log("File: ${file.name} failed to parse: $throwable.") + } + @Throws(PostHogApiError::class, IOException::class) private fun batchEvents() { val files = takeFiles() @@ -175,14 +193,12 @@ internal class PostHogQueue( val event = config.serializer.deserialize(it.reader().buffered()) event?.let { theEvent -> events.add(theEvent) + } ?: run { + deleteFileSafely(file) } } } catch (e: Throwable) { - synchronized(dequeLock) { - deque.remove(file) - } - file.deleteSafely(config) - config.logger.log("File: ${file.name} failed to parse: $e.") + deleteFileSafely(file, e) } } diff --git a/posthog/src/main/java/com/posthog/internal/PostHogSendCachedEventsIntegration.kt b/posthog/src/main/java/com/posthog/internal/PostHogSendCachedEventsIntegration.kt index 9181bc66..9879c99b 100644 --- a/posthog/src/main/java/com/posthog/internal/PostHogSendCachedEventsIntegration.kt +++ b/posthog/src/main/java/com/posthog/internal/PostHogSendCachedEventsIntegration.kt @@ -79,11 +79,12 @@ internal class PostHogSendCachedEventsIntegration( event?.let { events.add(event) eventsCount++ + } ?: run { + removeFileSafely(iterator) } } } catch (e: Throwable) { - iterator.remove() - config.logger.log("Event failed to parse: $e.") + removeFileSafely(iterator, e) } // stop the while loop since the batch is full if (events.size >= config.maxBatchSize) { @@ -135,6 +136,24 @@ internal class PostHogSendCachedEventsIntegration( } } + private fun deleteFileSafely( + file: File, + iterator: MutableIterator, + throwable: Throwable? = null, + ) { + config.logger.log("File: ${file.name} failed to parse: $throwable.") + iterator.remove() + file.deleteSafely(config) + } + + private fun removeFileSafely( + iterator: MutableIterator, + throwable: Throwable? = null, + ) { + config.logger.log("Event failed to parse: $throwable.") + iterator.remove() + } + @Throws(PostHogApiError::class, IOException::class) private fun flushEvents( storagePrefix: String?, @@ -175,12 +194,12 @@ internal class PostHogSendCachedEventsIntegration( event?.let { events.add(event) eventsCount++ + } ?: run { + deleteFileSafely(file, iterator) } } } catch (e: Throwable) { - config.logger.log("File: ${file.name} failed to parse: $e.") - iterator.remove() - file.deleteSafely(config) + deleteFileSafely(file, iterator, e) } // stop the while loop since the batch is full From 374d82d9f556b45dd7dda603de8870ee49c0e1dc Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 10 Mar 2025 11:21:46 +0100 Subject: [PATCH 2/3] pr id --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8c86f58d..e12d51b2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ ## Next - feat: support reuse of `anonymousId` between user changes ([#229](https://github.com/PostHog/posthog-android/pull/229)) -- chore: delete queue files if serialization fail +- chore: delete queue files if serialization fails ([#232](https://github.com/PostHog/posthog-android/pull/232)) ## 3.11.3 - 2025-02-26 From ab8e228df0b30808d7abd2e9c0be9e52c194a7b6 Mon Sep 17 00:00:00 2001 From: Manoel Aranda Neto Date: Mon, 10 Mar 2025 11:32:33 +0100 Subject: [PATCH 3/3] review --- .../src/main/java/com/posthog/internal/PostHogQueue.kt | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt b/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt index df47fdbc..14ce9bec 100644 --- a/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt +++ b/posthog/src/main/java/com/posthog/internal/PostHogQueue.kt @@ -81,24 +81,20 @@ internal class PostHogQueue( deque.add(file) } - var hasPendingEvents = true try { val os = config.encryption?.encrypt(file.outputStream()) ?: file.outputStream() os.use { theOutputStream -> config.serializer.serialize(event, theOutputStream.writer().buffered()) } config.logger.log("Queued Event ${event.event}: ${file.name}.") + + flushIfOverThreshold() } catch (e: Throwable) { - hasPendingEvents = false config.logger.log("Event ${event.event}: ${file.name} failed to parse: $e.") // if for some reason the file failed to serialize, lets delete it file.deleteSafely(config) } - - if (hasPendingEvents) { - flushIfOverThreshold() - } } } }