Skip to content

Initialize Okhttp and Retrofit in background#7042

Open
TimoPtr wants to merge 6 commits into
mainfrom
feature/background_okhttp_retrofit_init
Open

Initialize Okhttp and Retrofit in background#7042
TimoPtr wants to merge 6 commits into
mainfrom
feature/background_okhttp_retrofit_init

Conversation

@TimoPtr

@TimoPtr TimoPtr commented Jun 17, 2026

Copy link
Copy Markdown
Member

Summary

In #7038 we hit the fact that we are currently loading okhttp on the main thread, because we might load a keystore from the disk which is wrong. Instantiating OkHttp and Retrofit can be expensive and our Application is experiencing some ANR at application start already.
image

This PR move the initialization out of the main thread by using the SuspendProvider it has a lot of implications because we need OkHttp/Retrofit in many places and the usage of it also cascade into the DI graph so we need to adjust the factories to work with the fact that it requires a coroutineScope now.

One issue popped from the robolectric test of the onboarding because it was trying to provide the deviceID that is not available. I've made a quick fix but I'm pretty sure that we will revisit it later when more issues comes in.

I had to use another method to initialize Coil in an async manner SingletonImageLoader.setSafe

Call SingletonImageLoader.setSafe near the entrypoint to your app (e.g. in Application.onCreate on Android). This is the most flexible.

Checklist

  • New or updated tests have been added to cover the changes following the testing guidelines.
  • The code follows the project's code style and best_practices.
  • The changes have been thoroughly tested, and edge cases have been considered.
  • Changes are backward compatible whenever feasible. Any breaking changes are documented in the changelog for users and/or in the code for developers depending on the relevance.

Any other notes

I would like 1 week of beta of this, since it might have some impacts.

Copilot AI review requested due to automatic review settings June 17, 2026 18:00

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors HTTP stack initialization so OkHttp/Retrofit (and dependent services) can be initialized asynchronously via SuspendProvider, aiming to reduce main-thread work during app startup and avoid ANRs caused by disk-backed TLS/keystore setup.

Changes:

  • Introduces suspendable, mutex-guarded lazy initialization for OkHttpClient and Retrofit in HomeAssistantApis, and rewires DI to expose these via SuspendProvider.
  • Updates repositories/factories and various call sites (notifications, wear tiles, ExoPlayer data sources, wear settings) to consume suspend-provided OkHttp/Retrofit services.
  • Adjusts Robolectric onboarding tests by seeding Settings.Secure.ANDROID_ID, and changes Coil singleton initialization to use SingletonImageLoader.setSafe.

Reviewed changes

Copilot reviewed 23 out of 23 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
wear/src/main/kotlin/io/homeassistant/companion/android/tiles/CameraTile.kt Switches to suspend-provided OkHttp for fetching tile snapshot images
common/src/test/kotlin/io/homeassistant/companion/android/common/data/authentication/ServerRegistrationRepositoryTest.kt Updates test construction to match SuspendProvider-based auth service
common/src/main/kotlin/io/homeassistant/companion/android/di/DataModule.kt Replaces eager singletons with SuspendProvider bindings for HTTP/services and data source factory
common/src/main/kotlin/io/homeassistant/companion/android/common/util/UtilModule.kt Injects SuspendProvider<DataSource.Factory> into AudioUrlPlayer
common/src/main/kotlin/io/homeassistant/companion/android/common/util/ExoPlayerExt.kt Updates mTLS-aware data source factory to take a concrete OkHttp client
common/src/main/kotlin/io/homeassistant/companion/android/common/util/AudioUrlPlayer.kt Defers DataSource.Factory acquisition via SuspendProvider
common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/WebSocketRepository.kt Makes repository factory creation suspendable
common/src/main/kotlin/io/homeassistant/companion/android/common/data/websocket/WebSocketCore.kt Makes core factory creation suspendable and uses suspend-provided OkHttp
common/src/main/kotlin/io/homeassistant/companion/android/common/data/integration/IntegrationRepository.kt Replaces assisted factory with injected, suspendable factory pattern
common/src/main/kotlin/io/homeassistant/companion/android/common/data/HomeAssistantApis.kt Adds async, mutex-guarded lazy initialization for OkHttp/Retrofit
common/src/main/kotlin/io/homeassistant/companion/android/common/data/authentication/ServerRegistrationRepository.kt Switches auth calls to use suspend-provided AuthenticationService/installId
common/src/main/kotlin/io/homeassistant/companion/android/common/data/authentication/impl/AuthenticationRepositoryImpl.kt Removes assisted injection, aligns construction with new factory approach
common/src/main/kotlin/io/homeassistant/companion/android/common/data/authentication/AuthenticationRepository.kt Updates factory to inject suspend-provided service and installId
app/src/test/kotlin/io/homeassistant/companion/android/settings/wear/SettingsWearRepositoryTest.kt Updates test wiring for provider-based services
app/src/test/kotlin/io/homeassistant/companion/android/onboarding/WearOnboardingNavigationTest.kt Seeds ANDROID_ID in Robolectric setup to avoid DI crash
app/src/test/kotlin/io/homeassistant/companion/android/onboarding/BaseOnboardingNavigationTest.kt Seeds ANDROID_ID in shared Robolectric setup to avoid DI crash
app/src/main/kotlin/io/homeassistant/companion/android/widgets/camera/CameraWidget.kt Removes direct OkHttp injection (presumably moved behind other abstractions)
app/src/main/kotlin/io/homeassistant/companion/android/webview/WebViewActivity.kt Uses suspend-provided DataSource.Factory for ExoPlayer initialization
app/src/main/kotlin/io/homeassistant/companion/android/notifications/MessagingManager.kt Uses suspend-provided OkHttp for downloading notification media
app/src/main/kotlin/io/homeassistant/companion/android/HomeAssistantApplication.kt Initializes Coil singleton ImageLoader asynchronously using suspend-provided OkHttp
app/src/main/kotlin/io/homeassistant/companion/android/frontend/exoplayer/FrontendExoPlayerManager.kt Uses suspend-provided DataSource.Factory to build ExoPlayer
app/src/full/kotlin/io/homeassistant/companion/android/settings/wear/SettingsWearRepository.kt Migrates wear repository calls to provider-based Retrofit services
app/src/debug/kotlin/io/homeassistant/companion/android/developer/DemoExoPlayerActivity.kt Defers data source factory creation in Compose using produceState
Comments suppressed due to low confidence (1)

app/src/main/kotlin/io/homeassistant/companion/android/notifications/MessagingManager.kt:1514

  • The video download path reads from response.body.source() without null-checking body and without closing the Response, which can crash (null body) and leak sockets/file descriptors.
                    val response = okHttpClientProvider().newCall(request).execute()

                    if (!videoFile.exists()) {
                        videoFile.parentFile?.mkdirs()
                        videoFile.createNewFile()

Comment on lines 134 to 141
@SuppressLint("HardwareIds")
@Provides
@NamedDeviceId
@Singleton
fun provideDeviceId(@ApplicationContext appContext: Context) = Settings.Secure.getString(
fun provideDeviceId(@ApplicationContext appContext: Context): String = Settings.Secure.getString(
appContext.contentResolver,
Settings.Secure.ANDROID_ID,
)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I figure it out while working on this.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't know what you mean here but the suggestion is correct IMO (type can be inferred)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I had a IDE warning, also in our codebase we assume it's non null.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

What I've figured out is that within test this is not set so it has to be set manually otherwise the DI crashes.

@jpelgrom jpelgrom left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Implicitly tested as part of #7038, and it seems to resolve the described issue. However I'd you want to keep this in beta for a week I'd rebase #7038 so it can be merged cleanly and we can get a new stable release out first.

@TimoPtr TimoPtr force-pushed the feature/background_okhttp_retrofit_init branch from df38382 to 96808b3 Compare June 18, 2026 06:24
@TimoPtr TimoPtr changed the base branch from main to feature/use_default_keystore_and_fallback_to_user June 18, 2026 06:26
@TimoPtr TimoPtr force-pushed the feature/background_okhttp_retrofit_init branch from 2befcd5 to 8ed0a77 Compare June 18, 2026 08:11
Base automatically changed from feature/use_default_keystore_and_fallback_to_user to main June 18, 2026 09:15
@TimoPtr TimoPtr force-pushed the feature/background_okhttp_retrofit_init branch from 8ed0a77 to a1949e5 Compare June 18, 2026 09:19
@dominik-korsa

dominik-korsa commented Jun 19, 2026

Copy link
Copy Markdown

Possibly related: #6119.
That issue mentions a load issue on app launch, likely caused by a race condition when initializing KeyChainRepository in a coroutine. See this comment: #6119 (comment)
The implementation of the asynchronous HTTP client initialization in this PR could probably also be a fix for #6119.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants