Fail a sync none of whose events can be read and log the skipped ones… · GetStream/stream-chat-android@f1bb87c · GitHub
Skip to content

Commit f1bb87c

Browse files
committed
Fail a sync none of whose events can be read and log the skipped ones once per sync
1 parent 47ab032 commit f1bb87c

5 files changed

Lines changed: 83 additions & 83 deletions

File tree

‎stream-chat-android-client/src/main/java/io/getstream/chat/android/client/internal/state/sync/internal/SyncManager.kt‎

Lines changed: 4 additions & 4 deletions

‎stream-chat-android-client/src/main/java/io/getstream/chat/android/client/parser2/V2EventParser.kt‎

Lines changed: 27 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -127,10 +127,24 @@ internal class V2EventParser(
127127
* Parses the events a `/sync` response replays, each as the socket delivers it, leaving out those the SDK
128128
* ignores. An event that fails to parse is skipped and the rest are replayed: failing the response would keep the
129129
* sync cursor where it is, so every later sync would ask for the same events and fail on the same one. A response
130-
* whose envelope can't be read still fails.
130+
* whose envelope can't be read, or of whose events none can be read, still fails.
131131
*/
132132
fun parseSyncEvents(body: BufferedSource): List<ChatEvent> {
133133
val parsed = mutableListOf<ChatEvent>()
134+
val skipped = mutableListOf<SkippedEvent>()
135+
var read = 0
136+
forEachSyncEvent(body) { raw -> if (parseSyncEvent(raw, parsed, skipped)) read++ }
137+
if (skipped.isNotEmpty()) {
138+
val cause = skipped.first().cause
139+
// With nothing read, the failure is likely not one event's, and the cursor must not move past them all.
140+
if (read == 0) throw JsonDataException("None of the ${skipped.size} synced events could be read", cause)
141+
logger.e(cause) { "[parseSyncEvents] skipped ${skipped.size} event(s): ${skipped.map { it.type }}" }
142+
}
143+
return parsed
144+
}
145+
146+
/** Calls [action] with the raw JSON of each event in a `/sync` response's `events`. */
147+
private fun forEachSyncEvent(body: BufferedSource, action: (String) -> Unit) {
134148
JsonReader.of(body).use { reader ->
135149
reader.beginObject()
136150
while (reader.hasNext()) {
@@ -139,24 +153,29 @@ internal class V2EventParser(
139153
continue
140154
}
141155
reader.beginArray()
142-
while (reader.hasNext()) parseSyncEvent(reader.nextSource().use { it.readUtf8() })?.let(parsed::add)
156+
while (reader.hasNext()) action(reader.nextSource().use { it.readUtf8() })
143157
reader.endArray()
144158
}
145159
reader.endObject()
146160
}
147-
return parsed
148161
}
149162

163+
/**
164+
* Reads [raw] into [parsed], unless the SDK ignores it, and returns true; an event that can't be read is added to
165+
* [skipped] instead.
166+
*/
150167
@Suppress("TooGenericExceptionCaught")
151-
private fun parseSyncEvent(raw: String): ChatEvent? = try {
152-
parse(raw)
168+
private fun parseSyncEvent(raw: String, parsed: MutableList<ChatEvent>, skipped: MutableList<SkippedEvent>) = try {
169+
parse(raw)?.let(parsed::add)
170+
true
153171
} catch (e: Exception) {
154172
// Any failure to read one event skips it; an Error, such as running out of memory, still fails the sync.
155-
val type = runCatching { readHead(raw).type }.getOrNull() ?: "unknown"
156-
logger.e(e) { "[parseSyncEvents] skipped a '$type' event" }
157-
null
173+
skipped += SkippedEvent(runCatching { readHead(raw).type }.getOrNull() ?: "unknown", e)
174+
false
158175
}
159176

177+
private class SkippedEvent(val type: String, val cause: Exception)
178+
160179
private fun withCustomAtRoot(event: String): String {
161180
val fields = rawFields(event)
162181
val custom = fields["custom"]?.takeIf { it.trimStart().startsWith("{") }?.let(::rawFields).orEmpty()

‎stream-chat-android-client/src/test/java/io/getstream/chat/android/client/api2/MoshiChatApiTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2962,11 +2962,11 @@ internal class MoshiChatApiTest {
29622962
@Test
29632963
fun `A synced event that can't be read is skipped and the rest are replayed`() = runTest {
29642964
val api = mock<SyncApi>()
2965-
whenever(api.sync(any(), any()))
2965+
whenever(api.sync(any(), any(), any()))
29662966
.doReturn(RetroSuccess(SYNCED_UNREADABLE_EVENT.toResponseBody()).toRetrofitCall())
29672967
val sut = Fixture().withSyncApi(api).get()
29682968

2969-
val result = sut.getSyncHistory(listOf("messaging:general"), "2026-10-14T11:13:25.614650759Z").await()
2969+
val result = sut.getSyncHistory(listOf("messaging:general"), "2026-10-14T11:13:25.614650759Z", watch = false).await()
29702970

29712971
val event = (result as Result.Success).value.single() as UnknownEvent
29722972
event.type `should be equal to` "friendship_request"

‎stream-chat-android-client/src/test/java/io/getstream/chat/android/client/internal/state/internal/SyncManagerTest.kt‎

Lines changed: 33 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -178,12 +178,40 @@ internal class SyncManagerTest {
178178

179179
@ParameterizedTest
180180
@CsvSource(
181-
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:25.614650000Z",
182-
"2026-10-14T11:13:25Z, 2026-10-14T11:13:25.000000000Z",
181+
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:25.614650000Z, true",
182+
"2026-10-14T11:13:25Z, 2026-10-14T11:13:25.000000000Z, true",
183+
"2026-10-14T11:13:25.614650759Z, 2026-10-14T11:13:25.614650759Z, true",
184+
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:25.614651000Z, false",
185+
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:26.61465Z, false",
186+
// Dates that don't parse are compared as written.
187+
", , true",
188+
"not a date, not a date, true",
189+
"2026-10-14T11:13:25Z, , false",
190+
)
191+
fun `sameInstant compares raw dates whatever the precision of their fraction`(
192+
raw: String?,
193+
other: String?,
194+
expected: Boolean,
195+
) {
196+
assertEquals(expected, sameInstant(raw, other))
197+
assertEquals(expected, sameInstant(other, raw))
198+
}
199+
200+
/**
201+
* The SDK must not handle the event at the sync cursor on every sync: it wastes resources, and some events are not
202+
* idempotent. The cursor's date can be written with another precision than the replayed event's.
203+
*/
204+
@ParameterizedTest
205+
@CsvSource(
206+
"2026-10-14T11:13:25.614650759Z, 2026-10-14T11:13:25.614650759Z, false",
207+
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:25.614650000Z, false",
208+
"2026-10-14T11:13:25Z, 2026-10-14T11:13:25.000000000Z, false",
209+
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:25.614651000Z, true",
183210
)
184-
fun `an event at the cursor's instant is not propagated when its date is written with a different precision`(
211+
fun `a synced event is propagated only when it is not at the cursor's instant`(
185212
rawLastSyncedAt: String,
186213
rawCreatedAt: String,
214+
propagated: Boolean,
187215
) = runTest(testDispatcher) {
188216
val createdAt = localDate()
189217
val testSyncState = SyncState(
@@ -201,7 +229,7 @@ internal class SyncManagerTest {
201229
whenever(repositoryFacade.selectMessages(any())) doReturn listOf(randomMessage())
202230
whenever(repositoryFacade.selectChannels(any())) doReturn listOf(randomChannel())
203231
whenever(repositoryFacade.selectSyncState(any())) doReturn testSyncState
204-
whenever(chatClient.getSyncHistory(any(), any<String>())) doReturn TestCall(
232+
whenever(chatClient.getSyncHistoryInternal(any(), any<String>(), any())) doReturn TestCall(
205233
Result.Success(listOf(mockedChatEvent)),
206234
)
207235
val connectingEvent = ConnectedEvent(
@@ -215,74 +243,10 @@ internal class SyncManagerTest {
215243
_syncEvents.test {
216244
syncManager.onEvent(connectingEvent)
217245

218-
expectNoEvents()
246+
if (propagated) assertEquals(listOf(mockedChatEvent), awaitItem()) else expectNoEvents()
219247
}
220248
}
221249

222-
@ParameterizedTest
223-
@CsvSource(
224-
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:25.614650000Z, true",
225-
"2026-10-14T11:13:25Z, 2026-10-14T11:13:25.000000000Z, true",
226-
"2026-10-14T11:13:25.614650759Z, 2026-10-14T11:13:25.614650759Z, true",
227-
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:25.614651000Z, false",
228-
"2026-10-14T11:13:25.61465Z, 2026-10-14T11:13:26.61465Z, false",
229-
)
230-
fun `sameInstant compares raw dates whatever the precision of their fraction`(
231-
raw: String,
232-
other: String,
233-
expected: Boolean,
234-
) {
235-
assertEquals(expected, sameInstant(raw, other))
236-
assertEquals(expected, sameInstant(other, raw))
237-
}
238-
239-
@Test
240-
fun `when one event of exact same raw time of last sync arrive, it should not be propagated`() =
241-
runTest(testDispatcher) {
242-
/*
243-
* This checks if the SDK is avoiding loops in the sync. We don't want to handle the same event on every sync,
244-
* because this can waste resource and/or some events may not be idempotent.
245-
*/
246-
val createdAt = localDate()
247-
val rawCreatedAt = streamDateFormatter.format(createdAt)
248-
val testSyncState = SyncState(
249-
userId = randomString(),
250-
activeChannelIds = emptyList(),
251-
lastSyncedAt = createdAt,
252-
rawLastSyncedAt = rawCreatedAt,
253-
markedAllReadAt = createdAt,
254-
)
255-
256-
val syncManager = buildSyncManager()
257-
258-
val mockedChatEvent: ChatEvent = mock {
259-
on(it.createdAt) doReturn createdAt
260-
on(it.rawCreatedAt) doReturn rawCreatedAt
261-
}
262-
263-
whenever(repositoryFacade.selectMessages(any())) doReturn listOf(randomMessage())
264-
whenever(repositoryFacade.selectChannels(any())) doReturn listOf(randomChannel())
265-
whenever(repositoryFacade.selectSyncState(any())) doReturn testSyncState
266-
267-
whenever(chatClient.getSyncHistoryInternal(any(), any<String>(), any())) doReturn TestCall(
268-
Result.Success(listOf(mockedChatEvent)),
269-
)
270-
val connectingEvent = ConnectedEvent(
271-
type = "type",
272-
createdAt = createdAt,
273-
rawCreatedAt = rawCreatedAt,
274-
connectionId = randomString(),
275-
me = randomUser(),
276-
)
277-
278-
_syncEvents.test {
279-
syncManager.onEvent(connectingEvent)
280-
281-
expectNoEvents()
282-
}
283-
verify(chatClient).getSyncHistoryInternal(any(), eq(rawCreatedAt), any())
284-
}
285-
286250
@Test
287251
fun `test initial syncing when rawLastSyncedAt is null`() = runTest(testDispatcher) {
288252
/* Given */

‎stream-chat-android-client/src/test/java/io/getstream/chat/android/client/parser2/V2EventParserTest.kt‎

Lines changed: 17 additions & 0 deletions

0 commit comments

Comments
 (0)