fix(service): resolve UninitializedPropertyAccessException in onGetSession

[kairosci]

Problem

Users were experiencing app crashes and playback failures due to an UninitializedPropertyAccessException in MusicService.onGetSession.

Cause

MusicService.onCreate() could legitimately abort early due to ensureStartedAsForegroundOrStop(). When this happens, mediaSession (a lateinit var) was never initialized. However, the service could still be bound by Media3, causing onGetSession to be called and throwing the crash when trying to access the uninitialized property.

Solution

• Changed mediaSession from lateinit var to a properly modelled MediaLibrarySession? = null.
• Added safe call operators (?.) across all usages of mediaSession.
• onGetSession now safely returns null when the service is stopped prematurely, correctly signaling Media3 to reject the controller connection without crashing.

Testing

• Verified successful compilation (./gradlew :app:assembleFossDebug).
• Confirmed correct null-safety semantics according to the Media3 library specifications.

Related Issues

• Closes UninitializedPropertyAccessException in MusicService.onGetSession causing crashes and playback failure #3811

Summary by CodeRabbit

• Bug Fixes
  • Enhanced stability of the music playback service through improved session state management and more robust lifecycle handling. This prevents potential crashes that could occur during player transitions, audio preference changes, notification display updates, and service shutdown operations. Playback reliability is now improved across all scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR addresses a race condition where mediaSession is accessed before being fully initialized. The change converts mediaSession from a non-null lateinit field to a nullable MediaLibrarySession? property, and guards all session operations with null-safe access patterns to prevent UninitializedPropertyAccessException crashes.

Changes

MediaSession Null-Safety Initialization

Layer / File(s)	Summary
MediaSession null-safety refactoring app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt	Declares mediaSession as nullable MediaLibrarySession? and adds private var isRunning state flag. Updates five session operation sites—AudioTrack player recreation, notification custom layout, crossfade player swapping, and destruction—to use null-safe access (mediaSession?.let { } or mediaSession?.method()) instead of unsafe casts or unconditional calls.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Suggested reviewers

• nyxiereal

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main fix: resolving UninitializedPropertyAccessException in onGetSession, which is the core issue addressed by making mediaSession nullable and applying safe-call operators.
Description check	✅ Passed	The description follows the required template with all key sections completed: Problem, Cause, Solution, Testing, and Related Issues are all present and well-documented.
Linked Issues check	✅ Passed	The PR fully addresses issue #3811 by converting mediaSession from lateinit to nullable, applying safe-call operators, and ensuring onGetSession returns null when the service is not fully initialized, which prevents UninitializedPropertyAccessException.
Out of Scope Changes check	✅ Passed	All changes are directly focused on fixing the UninitializedPropertyAccessException in onGetSession through null-safety improvements; no unrelated or out-of-scope changes are present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (2)

app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt (2)

910-910: 💤 Low value

Consider simplifying the player assignment pattern.

The current pattern mediaSession?.let { (it as MediaSession).player = newPlayer } requires a downcast from MediaLibrarySession to MediaSession to access the player property setter. While this is safe (MediaLibrarySession extends MediaSession) and already wrapped in try-catch blocks, the cast is somewhat awkward. If this pattern becomes more common, consider extracting it into a small helper extension function to improve readability:

private fun MediaLibrarySession?.setPlayer(player: ExoPlayer) {
    this?.let { (it as MediaSession).player = player }
}

Then usage becomes: mediaSession.setPlayer(newPlayer).

Also applies to: 4406-4406

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt` at line
910, The mediaSession?.let { (it as MediaSession).player = newPlayer } pattern
is verbose and repeated; add a small nullable extension function like
MediaLibrarySession?.setPlayer(player: ExoPlayer) that does the safe cast and
assignment ((this as? MediaSession)?.player = player) and replace direct usages
(e.g., mediaSession?.let { (it as MediaSession).player = newPlayer }) with
mediaSession.setPlayer(newPlayer) to improve readability and avoid repeated
casts; update all occurrences (including the one around symbol mediaSession and
any similar uses at the other noted location) to call the new setPlayer
extension.

---

374-374: 💤 Low value

Consider whether the isRunning flag is necessary.

The new isRunning flag is set in onCreate and onDestroy, but it's not visibly used within the provided code changes. Since mediaSession nullability now serves as a signal for whether the service initialized successfully, this additional flag may be redundant. If it's intended for external access or future defensive checks, consider documenting its purpose. Otherwise, it could be removed to reduce state tracking.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt` at line
374, The isRunning boolean field appears redundant because mediaSession
nullability already indicates initialization; remove the private var isRunning =
false declaration and any assignments to it in onCreate and onDestroy (or, if
external visibility is required, replace it with a documented accessor that
derives state from mediaSession != null). Locate usages of isRunning in
MusicService, onCreate, and onDestroy and delete or refactor them to use
mediaSession != null, and add a brief KDoc to the service if you keep a derived
isRunning accessor.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt`:
- Line 910: The mediaSession?.let { (it as MediaSession).player = newPlayer }
pattern is verbose and repeated; add a small nullable extension function like
MediaLibrarySession?.setPlayer(player: ExoPlayer) that does the safe cast and
assignment ((this as? MediaSession)?.player = player) and replace direct usages
(e.g., mediaSession?.let { (it as MediaSession).player = newPlayer }) with
mediaSession.setPlayer(newPlayer) to improve readability and avoid repeated
casts; update all occurrences (including the one around symbol mediaSession and
any similar uses at the other noted location) to call the new setPlayer
extension.
- Line 374: The isRunning boolean field appears redundant because mediaSession
nullability already indicates initialization; remove the private var isRunning =
false declaration and any assignments to it in onCreate and onDestroy (or, if
external visibility is required, replace it with a documented accessor that
derives state from mediaSession != null). Locate usages of isRunning in
MusicService, onCreate, and onDestroy and delete or refactor them to use
mediaSession != null, and add a brief KDoc to the service if you keep a derived
isRunning accessor.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 2eeb0994-2bf8-42e3-b47a-66df195b81d0

📥 Commits

Reviewing files that changed from the base of the PR and between a6ff297 and 50939ab.

📒 Files selected for processing (1)

• app/src/main/kotlin/com/metrolist/music/playback/MusicService.kt

[kairosci]

I believe it is ready to review

cc: @Dr-Brixx @nyxiereal