Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes crashes when a timeline loader is not supported by introducing a NotSupported state and handling it gracefully in the presenter.
- Introduces a
NotSupportedcase inBaseTimelineLoaderand returns an emptyPagingDatawith error states. - Updates
discoverStatuses()exception message inXQTDataSourceto reference XQT. - Removes the debug
catch { printStackTrace() }and adds upstream error handling inTimelinePresenter.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| TimelinePresenter.kt | Wraps loader in a .catch that emits NotSupported, handles it by emitting an empty paging stream, and removes the old debug catch. |
| XQTDataSource.kt | Changes the unsupported operation exception message to "XQT does not support discover statuses". |
| BaseRemoteMediator.kt | Converts BaseTimelineLoader into a sealed interface with a data object NotSupported variant. |
| ) | ||
|
|
||
| BaseTimelineLoader.NotSupported -> | ||
| flowOf( |
There was a problem hiding this comment.
Using flowOf(...) produces a finite flow that emits once and then completes. When combined with filterFlow, the overall stream will complete early and stop responding to further filter updates. Consider returning a non-terminating flow (e.g., mapping filterFlow to PagingData.empty(...)) or restructuring so the combined flow continues emitting on filter changes.
| flowOf( | |
| filterFlow.map { |
|
|
||
| internal sealed interface BaseTimelineLoader | ||
| internal sealed interface BaseTimelineLoader { | ||
| data object NotSupported : BaseTimelineLoader |
There was a problem hiding this comment.
The data object syntax for NotSupported is experimental in Kotlin 1.9 and requires opt-in to ExperimentalStdlibApi. Please add @OptIn(ExperimentalStdlibApi::class) or replace with a regular object to avoid relying on experimental features.
| data object NotSupported : BaseTimelineLoader | |
| object NotSupported : BaseTimelineLoader |
| .catch { | ||
| emit(BaseTimelineLoader.NotSupported) |
There was a problem hiding this comment.
Catching all exceptions on loader and emitting NotSupported may hide real errors. Consider catching only specific exception types (e.g., UnsupportedOperationException) or at least logging unexpected exceptions to avoid masking other failures.
| .catch { | |
| emit(BaseTimelineLoader.NotSupported) | |
| .catch { exception -> | |
| when (exception) { | |
| is UnsupportedOperationException -> emit(BaseTimelineLoader.NotSupported) | |
| else -> { | |
| println("Unexpected error in loader: ${exception.message}") | |
| throw exception | |
| } | |
| } |
No description provided.