Handle unresolvable attachments in picker#6285
Merged
VelikovPetar merged 1 commit intodevelopfrom Mar 24, 2026
Merged
Conversation
Contributor
PR checklist ✅All required conditions are satisfied:
🎉 Great job! This PR is ready for review. |
Contributor
SDK Size Comparison 📏
|
- Update `StorageHelper` and `AttachmentMetaDataMapper` to safely handle cases where content URIs (e.g. cloud-backed files) cannot be opened. - Introduce `hasUnresolvedAttachments` state in `AttachmentsPickerViewModel` to track failed attachment resolutions. - Show a toast message in both View-based and Compose attachment pickers when files are unavailable and need to be downloaded to the device. - Add `clearUnresolvedAttachments` to reset the error state after it has been consumed by the UI. - Add unit tests for unresolved attachment scenarios in `AttachmentsPickerViewModelTest`.
andremion
force-pushed
the
AND-1103-google-drive-attachment-issue
branch
from
40ac853 to
7678df3
Compare
|
WalkthroughThe changes introduce error handling for attachments that cannot be resolved during the attachment picker flow. When attachment resolution fails, the system sets a flag, displays a user-facing error toast, filters out unresolvable items, and clears the flag. Storage layer operations now gracefully return null for unresolvable attachments instead of propagating exceptions. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant AP as AttachmentsPicker
participant VM as AttachmentsPickerViewModel
participant SHW as StorageHelperWrapper
participant SH as StorageHelper
User->>AP: Select attachments
AP->>VM: getSelectedAttachments()
VM->>SHW: getAttachmentsForUpload(metadata)
SHW->>SH: mapNotNull - getCachedFileFromUri(each metadata)
SH-->>SHW: File or null (with error logging if failure)
SHW-->>VM: Filtered Attachment list (smaller if any unresolvable)
VM->>VM: Detect: resolved count < input count
VM->>VM: Set hasUnresolvedAttachments = true
VM-->>AP: Return filtered attachments
AP->>AP: LaunchedEffect observes flag change
AP->>User: Show error toast (unresolvable attachments)
AP->>VM: clearUnresolvedAttachments()
VM->>VM: Reset flag to false
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (3)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentsPicker.kt (1)
108-120: Minor simplification: Use resource ID directly in Toast.
Toast.makeText()accepts a resource ID directly, socontext.getString()is unnecessary.✨ Suggested simplification
LaunchedEffect(hasUnresolvedAttachments) { if (hasUnresolvedAttachments) { Toast.makeText( context, - context.getString(R.string.stream_ui_attachment_picker_error_unresolvable_attachments), + R.string.stream_ui_attachment_picker_error_unresolvable_attachments, Toast.LENGTH_LONG, ).show() attachmentsPickerViewModel.clearUnresolvedAttachments() } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentsPicker.kt` around lines 108 - 120, The Toast creation in the LaunchedEffect uses context.getString(...) unnecessarily; update the Toast.makeText call in the AttachmentsPicker composable (where LocalContext.current, hasUnresolvedAttachments, and attachmentsPickerViewModel.clearUnresolvedAttachments are used) to pass the string resource ID (R.string.stream_ui_attachment_picker_error_unresolvable_attachments) directly instead of context.getString(...), leaving the context and Toast.LENGTH_LONG parameters unchanged.stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.kt (1)
32-32: Consider caching or injecting theStorageHelperinstance.A new
StorageHelper()is created on every call totoAttachment(). While likely not a performance concern for typical attachment counts, consider whether this mapper could receive a shared instance, especially if this method is called in a loop for many attachments.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.kt` at line 32, AttachmentMetaDataMapper currently constructs a new StorageHelper on every call to toAttachment (see the fileFromUri assignment), which can be optimized by injecting or reusing a shared StorageHelper instance; modify AttachmentMetaDataMapper to accept a StorageHelper via constructor (or provide a lazily initialized/shared instance) and replace the inline new StorageHelper() call in toAttachment with that injected/shared instance so repeated toAttachment calls reuse the same StorageHelper.stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.kt (1)
237-247: Thread-safety consideration forhasUnresolvedAttachmentsmodification.
getSelectedAttachments()is public and modifieshasUnresolvedAttachments. When called viagetSelectedAttachmentsAsync(), this modification occurs onDispatcherProvider.IO. While Compose'smutableStateOfis thread-safe for reads, writes should typically occur on the main thread.Consider moving the flag update to the main thread:
🔧 Suggested fix
internal fun getSelectedAttachmentsAsync(onComplete: (List<Attachment>) -> Unit) { viewModelScope.launch { - val attachments = withContext(DispatcherProvider.IO) { - getSelectedAttachments() + val dataSet = if (attachmentsPickerMode == Files) files else images + val selectedMetaData = dataSet + .filter(AttachmentPickerItemState::isSelected) + .map(AttachmentPickerItemState::attachmentMetaData) + val attachments = withContext(DispatcherProvider.IO) { + storageHelper.getAttachmentsForUpload(selectedMetaData) + } + if (attachments.size < selectedMetaData.size) { + hasUnresolvedAttachments = true } onComplete(attachments) } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.kt` around lines 237 - 247, getSelectedAttachments() computes attachments off the IO dispatcher (via getSelectedAttachmentsAsync()) but directly writes to the mutable state hasUnresolvedAttachments from that background context; move the state mutation to the main thread by performing only the expensive work (storageHelper.getAttachmentsForUpload and size comparison) on IO and then posting the result to the main dispatcher—e.g., after computing attachments and determining the boolean (unresolved = attachments.size < selectedMetaData.size), dispatch a coroutine on the main dispatcher (using viewModelScope and DispatcherProvider.main()/Dispatchers.Main) to set hasUnresolvedAttachments = unresolved; keep the rest of getSelectedAttachments/getSelectedAttachmentsAsync logic intact and only relocate the write to hasUnresolvedAttachments onto the main thread.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentsPicker.kt`:
- Around line 108-120: The Toast creation in the LaunchedEffect uses
context.getString(...) unnecessarily; update the Toast.makeText call in the
AttachmentsPicker composable (where LocalContext.current,
hasUnresolvedAttachments, and
attachmentsPickerViewModel.clearUnresolvedAttachments are used) to pass the
string resource ID
(R.string.stream_ui_attachment_picker_error_unresolvable_attachments) directly
instead of context.getString(...), leaving the context and Toast.LENGTH_LONG
parameters unchanged.
In
`@stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.kt`:
- Around line 237-247: getSelectedAttachments() computes attachments off the IO
dispatcher (via getSelectedAttachmentsAsync()) but directly writes to the
mutable state hasUnresolvedAttachments from that background context; move the
state mutation to the main thread by performing only the expensive work
(storageHelper.getAttachmentsForUpload and size comparison) on IO and then
posting the result to the main dispatcher—e.g., after computing attachments and
determining the boolean (unresolved = attachments.size < selectedMetaData.size),
dispatch a coroutine on the main dispatcher (using viewModelScope and
DispatcherProvider.main()/Dispatchers.Main) to set hasUnresolvedAttachments =
unresolved; keep the rest of getSelectedAttachments/getSelectedAttachmentsAsync
logic intact and only relocate the write to hasUnresolvedAttachments onto the
main thread.
In
`@stream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.kt`:
- Line 32: AttachmentMetaDataMapper currently constructs a new StorageHelper on
every call to toAttachment (see the fileFromUri assignment), which can be
optimized by injecting or reusing a shared StorageHelper instance; modify
AttachmentMetaDataMapper to accept a StorageHelper via constructor (or provide a
lazily initialized/shared instance) and replace the inline new StorageHelper()
call in toAttachment with that injected/shared instance so repeated toAttachment
calls reuse the same StorageHelper.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: aee5ca77-d6f0-495d-8d07-35ff2bc36c5c
📒 Files selected for processing (8)
stream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/messages/attachments/AttachmentsPicker.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/ui/util/StorageHelperWrapper.ktstream-chat-android-compose/src/main/java/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModel.ktstream-chat-android-compose/src/test/kotlin/io/getstream/chat/android/compose/viewmodel/messages/AttachmentsPickerViewModelTest.ktstream-chat-android-ui-common/src/main/kotlin/io/getstream/chat/android/ui/common/helper/internal/StorageHelper.ktstream-chat-android-ui-common/src/main/res/values/strings.xmlstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/attachment/picker/AttachmentsPickerDialogFragment.ktstream-chat-android-ui-components/src/main/kotlin/io/getstream/chat/android/ui/feature/messages/composer/internal/AttachmentMetaDataMapper.kt
VelikovPetar
approved these changes
Mar 24, 2026
Contributor
|
🚀 Available in v6.36.0 |
andremion
added a commit
that referenced
this pull request
Apr 7, 2026
* Improve `Message.createdLocallyAt` creation logic using estimated server time (#6199) * Fix createdLocallyAt using NTP-style server clock offset estimation Co-Authored-By: Claude <noreply@anthropic.com> * Pr remarks * Adjust thread message createdLocallyAt. * Ensure exceedsSyncThreshold is compared against estimated server time (where applicable). * Add max allowed offset. --------- Co-authored-by: Claude <noreply@anthropic.com> * [skip ci] Update SDK sizes * Update README cover image (#6282) * Fix XML image flicker caused by `interceptorCoroutineContext(Dispatchers.IO)` (#6284) Co-authored-by: Claude <noreply@anthropic.com> * [skip ci] Update SDK sizes * AUTOMATION: Version Bump * Fix race condition in plugin resolution during disconnect (#6269) * Update `DependencyResolverTest` to verify error handling when dependency resolution races with disconnection. * Prevent race conditions during disconnects in `ChatClient`. * Handle unresolvable attachments in picker (#6285) - Update `StorageHelper` and `AttachmentMetaDataMapper` to safely handle cases where content URIs (e.g. cloud-backed files) cannot be opened. - Introduce `hasUnresolvedAttachments` state in `AttachmentsPickerViewModel` to track failed attachment resolutions. - Show a toast message in both View-based and Compose attachment pickers when files are unavailable and need to be downloaded to the device. - Add `clearUnresolvedAttachments` to reset the error state after it has been consumed by the UI. - Add unit tests for unresolved attachment scenarios in `AttachmentsPickerViewModelTest`. * [skip ci] Update SDK sizes * Fix wrong message selected on quoted message long click (#6292) * Use type-specific attachment URL fields and deprecate `imagePreviewUrl` (#6280) * Deprecate imagePreviewUrl and use type-specific attachment URL fields Co-Authored-By: Claude <noreply@anthropic.com> * Extract common extensions. --------- Co-authored-by: Claude <noreply@anthropic.com> * Expose optional completion callback for audio recording (#6290) Co-authored-by: Claude <noreply@anthropic.com> * AUTOMATION: Version Bump * AUTOMATION: Clean Detekt Baseline Files (#6299) Co-authored-by: adasiewiczr <17440581+adasiewiczr@users.noreply.github.com> * Add support for intercepting CDN file requests (#6295) * Add new CDN contract. * Add CDN for document files. * Add CDN support for downloading attachments. * Deprecate current CDN methods. * Add progress indicator snackbar. * Add useDocumentGView config flag. * Add file sharing cache handling. * Add file sharing cache handling. * Remove CDNResponse.kt * Add tests * PR remarks * [skip ci] Update SDK sizes * Post-merge clean-up. * Post-merge clean-up. * ApiDump. * Improve attachment URI resolution and error handling in `AttachmentsPickerViewModel` and `AttachmentStorageHelper`. - Add `isUriResolvable` to `StorageHelper` to verify if a content URI can be opened for reading. - Implement `partitionResolvable` in `AttachmentStorageHelper` to separate metadata based on URI accessibility. - Update `AttachmentsPickerViewModel.resolveAndSubmitUris` to exclude inaccessible URIs (e.g., undownloaded cloud files) from the submission. - Ensure `hasUnresolvedAttachments` is correctly set when URIs are inaccessible, independent of file type support. - Add unit tests in `AttachmentStorageHelperTest` and `AttachmentsPickerViewModelTest` to verify partitioning logic and view model state updates. * Handle unresolvable attachments in XML * apiDump. --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: André Mion <andremion@gmail.com> Co-authored-by: Gianmarco <47775302+gpunto@users.noreply.github.com> Co-authored-by: stream-pr-merger[bot] <117762243+stream-pr-merger[bot]@users.noreply.github.com> Co-authored-by: adasiewiczr <17440581+adasiewiczr@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Fix a crash (
FileNotFoundException) when users attach files from Google Drive or other cloud storage providers through the attachment picker. The content URI cannot always be resolved to a local input stream, causing an unhandled exception.Implementation
contentResolver.openInputStream()inStorageHelper.getCachedFileFromUriwith a try-catch so unresolvable URIs return null instead of crashing.StorageHelperWrapper) and UI Components (AttachmentMetaDataMapper) paths.Testing
AttachmentsPickerDialogFragment).Summary by CodeRabbit
New Features
Bug Fixes
Tests