Skip to content

Commit 56b0e7e

Browse files
matusekmaLasOriLordAndrasmegamegaxFarkasAndrasDev
committed
fix(remote-config): fix app code based remote config fetching error handling
ticket: SDK-827 epic: SDK-709 Co-authored-by: LasOri <24588073+LasOri@users.noreply.github.com> Co-authored-by: Andras Sarro <lordandras@users.noreply.github.com> Co-authored-by: megamegax <megamegax@users.noreply.github.com> Co-authored-by: Andras Farkas <224590688+FarkasAndrasDev@users.noreply.github.com>
1 parent 60dd451 commit 56b0e7e

File tree

2 files changed

+123
-53
lines changed

2 files changed

+123
-53
lines changed

‎engagement-cloud-sdk/src/commonMain/kotlin/com/sap/ec/networking/clients/remoteConfig/RemoteConfigClient.kt‎

Lines changed: 38 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package com.sap.ec.networking.clients.remoteConfig
33
import com.sap.ec.core.channel.SdkEventManagerApi
44
import com.sap.ec.core.crypto.CryptoApi
55
import com.sap.ec.core.db.events.EventsDaoApi
6+
import com.sap.ec.core.exceptions.SdkException
67
import com.sap.ec.core.log.Logger
78
import com.sap.ec.core.networking.clients.NetworkClientApi
89
import com.sap.ec.core.networking.model.UrlRequest
@@ -16,6 +17,7 @@ import com.sap.ec.networking.clients.error.ClientExceptionHandler
1617
import com.sap.ec.remoteConfig.RemoteConfigResponse
1718
import com.sap.ec.remoteConfig.RemoteConfigResponseHandlerApi
1819
import io.ktor.http.HttpMethod
20+
import io.ktor.http.HttpStatusCode
1921
import kotlinx.coroutines.CoroutineScope
2022
import kotlinx.coroutines.CoroutineStart
2123
import kotlinx.coroutines.async
@@ -52,22 +54,22 @@ internal class RemoteConfigClient(
5254
private suspend fun startEventConsumer() {
5355
sdkEventManager.onlineSdkEvents
5456
.filter { isRemoteConfigEvent(it) }
55-
.collect {
57+
.collect { event ->
5658
try {
5759
sdkLogger.debug("ConsumeRemoteConfigEvents")
58-
val remoteConfigResponse = fetchRemoteConfig(it)
60+
val remoteConfigResponse = fetchRemoteConfig(event)
5961
remoteConfigResponse?.let {
6062
remoteConfigResponseHandler.handle(remoteConfigResponse)
61-
}
62-
sdkEventManager.emitEvent(
63-
Response(
64-
originId = it.id,
65-
Result.success(remoteConfigResponse)
63+
sdkEventManager.emitEvent(
64+
Response(
65+
originId = event.id,
66+
Result.success(remoteConfigResponse)
67+
)
6668
)
67-
)
68-
it.ack(eventsDao, sdkLogger)
69+
event.ack(eventsDao, sdkLogger)
70+
}
6971
} catch (exception: Exception) {
70-
handleException(exception, it)
72+
handleException(exception, event)
7173
}
7274
}
7375
}
@@ -79,12 +81,26 @@ internal class RemoteConfigClient(
7981
event
8082
)
8183

82-
sdkEventManager.emitEvent(
83-
Response(
84-
originId = event.id,
85-
Result.failure<Exception>(exception)
84+
// TEMPORARY FIX UNTIL REMOTE CONFIG UPLOAD IS AUTOMATED FOR NEW APP CODES
85+
if (event is SdkEvent.Internal.Sdk.ApplyAppCodeBasedRemoteConfig &&
86+
exception is SdkException.FailedRequestException &&
87+
exception.response.status == HttpStatusCode.NotFound
88+
) {
89+
sdkLogger.debug("Remote config for app code not found.")
90+
sdkEventManager.emitEvent(
91+
Response(
92+
originId = event.id,
93+
Result.success(Unit)
94+
)
8695
)
87-
)
96+
} else {
97+
sdkEventManager.emitEvent(
98+
Response(
99+
originId = event.id,
100+
Result.failure<Exception>(exception)
101+
)
102+
)
103+
}
88104
}
89105

90106
private fun isRemoteConfigEvent(sdkEvent: SdkEvent): Boolean {
@@ -120,6 +136,13 @@ internal class RemoteConfigClient(
120136
if (verified) {
121137
json.decodeFromString(config)
122138
} else {
139+
event.ack(eventsDao, sdkLogger)
140+
sdkEventManager.emitEvent(
141+
Response(
142+
originId = event.id,
143+
Result.failure<Unit>(RuntimeException("Remote config signature verification failed"))
144+
)
145+
)
123146
null
124147
}
125148
}

‎engagement-cloud-sdk/src/commonTest/kotlin/com/sap/ec/networking/clients/remoteConfig/RemoteConfigClientTests.kt‎

Lines changed: 85 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ import dev.mokkery.answering.throws
2323
import dev.mokkery.every
2424
import dev.mokkery.everySuspend
2525
import dev.mokkery.matcher.any
26+
import dev.mokkery.matcher.capture.Capture
27+
import dev.mokkery.matcher.capture.capture
2628
import dev.mokkery.mock
2729
import dev.mokkery.resetAnswers
2830
import dev.mokkery.resetCalls
@@ -90,7 +92,7 @@ class RemoteConfigClientTests {
9092
mockEventsDao = mock(MockMode.autoUnit)
9193
mockClientExceptionHandler = mock(MockMode.autoUnit)
9294
mockSdkLogger = mock(MockMode.autofill)
93-
everySuspend { mockSdkLogger.error(any<String>(), any<Throwable>()) } returns Unit
95+
everySuspend { mockSdkLogger.error(any(), any<Throwable>()) } returns Unit
9496
}
9597

9698
@AfterTest
@@ -222,7 +224,12 @@ class RemoteConfigClientTests {
222224
)
223225
} returns Result.success(configSignatureResponse)
224226
everySuspend { mockCrypto.verify(any(), any()) } returns false
225-
227+
val eventSlot = Capture.slot<SdkEvent.Internal.Sdk.Answer.Response<Response>>()
228+
everySuspend {
229+
mockSdkEventManager.emitEvent(
230+
capture(eventSlot)
231+
)
232+
} returns Unit
226233

227234
val appCodeBasedRemoteConfigEvent = SdkEvent.Internal.Sdk.ApplyAppCodeBasedRemoteConfig()
228235

@@ -234,51 +241,82 @@ class RemoteConfigClientTests {
234241

235242
onlineSdkEvents.await() shouldBe listOf(appCodeBasedRemoteConfigEvent)
236243
verifySuspend(VerifyMode.exactly(0)) { mockRemoteConfigResponseHandler.handle(any()) }
237-
verifySuspend { mockEventsDao.removeEvent(appCodeBasedRemoteConfigEvent) }
244+
verifySuspend {
245+
appCodeBasedRemoteConfigEvent.ack(mockEventsDao, mockSdkLogger)
246+
}
247+
verifySuspend(VerifyMode.exactly(1)) {
248+
mockSdkEventManager.emitEvent(any())
249+
}
250+
eventSlot.values.size shouldBe 1
251+
eventSlot.values.first().let {
252+
it.originId shouldBe appCodeBasedRemoteConfigEvent.id
253+
it.result.isFailure shouldBe true
254+
it.result.exceptionOrNull()?.message shouldBe "Remote config signature verification failed"
255+
}
238256
}
239257

240258
@Test
241-
fun testConsumer_shouldNotCall_responseHandler_butAckEvent_whenConfigIsNotFound() = runTest {
242-
createClient(backgroundScope).register()
243-
244-
val configResponse =
245-
Response(configRequest, HttpStatusCode.NotFound, Headers.Empty, CONFIG_RESULT)
246-
val configSignatureResponse = Response(
247-
configSignatureRequest,
248-
HttpStatusCode.OK,
249-
Headers.Empty,
250-
CONFIG_SIGNATURE_RESULT
251-
)
259+
fun testConsumer_shouldNotCall_responseHandler_butAckEvent_andNotReemitEvent_whenConfigIsNotFound() =
260+
runTest {
261+
createClient(backgroundScope).register()
252262

253-
every { mockUrlFactory.create(ECUrlType.RemoteConfig) } returns configUrl
254-
every {
255-
mockUrlFactory.create(
256-
ECUrlType.RemoteConfigSignature
263+
val configResponse =
264+
Response(configRequest, HttpStatusCode.NotFound, Headers.Empty, CONFIG_RESULT)
265+
val configSignatureResponse = Response(
266+
configSignatureRequest,
267+
HttpStatusCode.OK,
268+
Headers.Empty,
269+
CONFIG_SIGNATURE_RESULT
257270
)
258-
} returns configSignatureUrl
259-
everySuspend { mockNetworkClient.send(configRequest) } returns Result.failure(
260-
SdkException.FailedRequestException(
271+
272+
every { mockUrlFactory.create(ECUrlType.RemoteConfig) } returns configUrl
273+
every {
274+
mockUrlFactory.create(
275+
ECUrlType.RemoteConfigSignature
276+
)
277+
} returns configSignatureUrl
278+
val exception = SdkException.FailedRequestException(
261279
configResponse
262280
)
263-
)
264-
everySuspend {
265-
mockNetworkClient.send(
266-
configSignatureRequest
281+
everySuspend { mockNetworkClient.send(configRequest) } returns Result.failure(
282+
exception
267283
)
268-
} returns Result.success(configSignatureResponse)
284+
everySuspend {
285+
mockNetworkClient.send(
286+
configSignatureRequest
287+
)
288+
} returns Result.success(configSignatureResponse)
269289

270-
val appCodeBasedRemoteConfigEvent = SdkEvent.Internal.Sdk.ApplyAppCodeBasedRemoteConfig()
290+
val appCodeBasedRemoteConfigEvent =
291+
SdkEvent.Internal.Sdk.ApplyAppCodeBasedRemoteConfig()
271292

272-
val onlineSdkEvents = backgroundScope.async {
273-
onlineEvents.take(1).toList()
274-
}
293+
val onlineSdkEvents = backgroundScope.async {
294+
onlineEvents.take(1).toList()
295+
}
275296

276-
onlineEvents.emit(appCodeBasedRemoteConfigEvent)
297+
onlineEvents.emit(appCodeBasedRemoteConfigEvent)
277298

278-
onlineSdkEvents.await() shouldBe listOf(appCodeBasedRemoteConfigEvent)
279-
verifySuspend(VerifyMode.exactly(0)) { mockRemoteConfigResponseHandler.handle(any()) }
280-
verifySuspend { mockEventsDao.removeEvent(appCodeBasedRemoteConfigEvent) }
281-
}
299+
onlineSdkEvents.await() shouldBe listOf(appCodeBasedRemoteConfigEvent)
300+
verifySuspend(VerifyMode.exactly(0)) { mockRemoteConfigResponseHandler.handle(any()) }
301+
verifySuspend {
302+
mockClientExceptionHandler.handleException(
303+
exception,
304+
any(),
305+
appCodeBasedRemoteConfigEvent
306+
)
307+
}
308+
verifySuspend {
309+
mockSdkEventManager.emitEvent(
310+
SdkEvent.Internal.Sdk.Answer.Response(
311+
appCodeBasedRemoteConfigEvent.id,
312+
Result.success(Unit)
313+
)
314+
)
315+
}
316+
verifySuspend(VerifyMode.exactly(0)) {
317+
mockSdkEventManager.emitEvent(appCodeBasedRemoteConfigEvent)
318+
}
319+
}
282320

283321
@Test
284322
fun testConsumer_shouldNotCall_responseHandler_butAckEvent_whenSignatureIsNotFound() = runTest {
@@ -300,11 +338,12 @@ class RemoteConfigClientTests {
300338
)
301339
} returns configSignatureUrl
302340
everySuspend { mockNetworkClient.send(configRequest) } returns Result.success(configResponse)
341+
val exception = SdkException.FailedRequestException(configSignatureResponse)
303342
everySuspend {
304343
mockNetworkClient.send(
305344
configSignatureRequest
306345
)
307-
} returns Result.failure(SdkException.FailedRequestException(configSignatureResponse))
346+
} returns Result.failure(exception)
308347

309348
val appCodeBasedRemoteConfigEvent = SdkEvent.Internal.Sdk.ApplyAppCodeBasedRemoteConfig()
310349

@@ -316,7 +355,13 @@ class RemoteConfigClientTests {
316355

317356
onlineSdkEvents.await() shouldBe listOf(appCodeBasedRemoteConfigEvent)
318357
verifySuspend(VerifyMode.exactly(0)) { mockRemoteConfigResponseHandler.handle(any()) }
319-
verifySuspend { mockEventsDao.removeEvent(appCodeBasedRemoteConfigEvent) }
358+
verifySuspend {
359+
mockClientExceptionHandler.handleException(
360+
exception,
361+
any(),
362+
appCodeBasedRemoteConfigEvent
363+
)
364+
}
320365
}
321366

322367
@Test
@@ -331,7 +376,9 @@ class RemoteConfigClientTests {
331376
every {
332377
mockUrlFactory.create(ECUrlType.RemoteConfig)
333378
} returns configUrl
334-
everySuspend { mockNetworkClient.send(configRequest) } returns Result.success(configResponse)
379+
everySuspend { mockNetworkClient.send(configRequest) } returns Result.success(
380+
configResponse
381+
)
335382
every {
336383
mockUrlFactory.create(ECUrlType.RemoteConfigSignature)
337384
} returns configSignatureUrl

0 commit comments

Comments
 (0)