Skip to content

Resolve crash that occurs when a connection to Keycloak fails #3822

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.

Sign up for GitHub

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

Open
allan-on wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Resolve crash that occurs when a connection to Keycloak fails #3822

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
3d5294d
:arrow_up: Update Data Capture Snapshot version
allan-on Jan 6, 2026
77ff7b4
:bug: Refactor fetchToken() exception handling
allan-on Jan 7, 2026
d5addab
Merge branch 'main' into bugfix/3821-fix-auth-server-connection-crash
allan-on Jan 16, 2026
4059877
Merge branch 'main' of github.com:opensrp/fhircore into bugfix/3821-f…
allan-on Jan 20, 2026
2550a29
:goal_net: Explicitly define some exceptions handled
allan-on Jan 20, 2026
0c2c826
:art: Apply spotless formatting
allan-on Jan 20, 2026
d8a9a17
Merge branch 'bugfix/3821-fix-auth-server-connection-crash' of github…
allan-on Jan 20, 2026
b7a08a2
Merge branch 'main' into bugfix/3821-fix-auth-server-connection-crash
allan-on Feb 4, 2026
b9828fc
Merge branch 'main' into bugfix/3821-fix-auth-server-connection-crash
dubdabasoduba Mar 24, 2026
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
9 changes: 2 additions & 7 deletions .../src/main/java/org/smartregister/fhircore/engine/data/remote/shared/TokenAuthenticator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ import java.net.UnknownHostException
import java.util.Base64
import javax.inject.Inject
import javax.inject.Singleton
import javax.net.ssl.SSLHandshakeException
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.withContext
import org.smartregister.fhircore.engine.configuration.app.ConfigService
Expand Down Expand Up @@ -163,12 +162,8 @@ constructor(
val oAuthResponse = oAuthService.fetchToken(body)
saveToken(username = username, password = password, oAuthResponse = oAuthResponse)
Result.success(oAuthResponse)
} catch (httpException: HttpException) {
Result.failure(httpException)
} catch (unknownHostException: UnknownHostException) {
Result.failure(unknownHostException)
} catch (sslHandShakeException: SSLHandshakeException) {
Result.failure(sslHandShakeException)
} catch (e: Exception) {
Result.failure(e)
Comment on lines 166 to +173
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

fetchAccessToken now catches Exception, which will also catch kotlinx.coroutines.CancellationException and prevent cooperative cancellation (and can cause structured-concurrency bugs). Consider adding a catch (e: CancellationException) { throw e } before this, and narrowing the generic catch to IOException (plus HttpException) to avoid masking programmer errors while still covering ConnectException, SocketTimeoutException, UnknownHostException, SSLHandshakeException, etc.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator

@ellykits ellykits Mar 25, 2026

Choose a reason for hiding this comment

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

@allan-on Can you apply this change it is okay.

}
Copy link
Copy Markdown
Collaborator

@ellykits ellykits Jan 8, 2026

Choose a reason for hiding this comment

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

Instead of omitting all these, you can retain them and add.

catch (e: Exception) {
  Result.failure(e)
}

At the end, to catch any missed exceptions.

}

Expand Down
39 changes: 15 additions & 24 deletions ...oid/engine/src/test/java/org/smartregister/fhircore/engine/auth/TokenAuthenticatorTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,35 +268,26 @@ class TokenAuthenticatorTest : RobolectricTest() {

@Test
@ExperimentalCoroutinesApi
fun testFetchTokenShouldShouldCatchHttpAndUnknownHostAndSSLHandshakeExceptions() {
fun testFetchAccessTokenShouldCatchAllNetworkExceptions() {
val username = sampleUsername
val password = charArrayOf('P', '4', '5', '5', 'W', '4', '0')

val httpException = HttpException(Response.success(null))

coEvery { oAuthService.fetchToken(any()) }.throws(httpException)

runTest {
var result = tokenAuthenticator.fetchAccessToken(username, password)
Assert.assertEquals(Result.failure<HttpException>(httpException), result)
}

val unknownHostException = UnknownHostException()

coEvery { oAuthService.fetchToken(any()) }.throws(unknownHostException)

runTest {
var result = tokenAuthenticator.fetchAccessToken(username, password)
Assert.assertEquals(Result.failure<UnknownHostException>(unknownHostException), result)
}

val sslHandshakeException = SSLHandshakeException("reason")
// Test with various exception types to verify generic exception handling
val exceptions = listOf(
HttpException(Response.error<OAuthResponse>(401, mockk(relaxed = true))),
UnknownHostException(),
SSLHandshakeException("SSL error"),
IOException("IO error")
)

coEvery { oAuthService.fetchToken(any()) }.throws(sslHandshakeException)
exceptions.forEach { exception ->
coEvery { oAuthService.fetchToken(any()) }.throws(exception)

runTest {
var result = tokenAuthenticator.fetchAccessToken(username, password)
Assert.assertEquals(Result.failure<SSLHandshakeException>(sslHandshakeException), result)
runTest {
val result = tokenAuthenticator.fetchAccessToken(username, password)
Assert.assertTrue(result.isFailure)
Assert.assertEquals(exception, result.exceptionOrNull())
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion android/gradle/libs.versions.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ espresso-core = "3.6.1"
fhir-sdk-common = "0.1.0-alpha05-preview3-SNAPSHOT"
fhir-sdk-contrib-barcode = "0.1.0-beta3-preview7-SNAPSHOT"
fhir-sdk-contrib-locationwidget = "0.1.0-alpha01-preview2-SNAPSHOT"
fhir-sdk-data-capture = "1.3.0-preview-SNAPSHOT"
fhir-sdk-data-capture = "1.3.0-preview1-SNAPSHOT"
fhir-sdk-engine = "1.2.0-preview-SNAPSHOT"
fhir-sdk-knowledge = "0.1.0-beta01-preview1-SNAPSHOT"
fhir-sdk-workflow = "0.1.0-beta01-preview2-SNAPSHOT"
Expand Down
Loading