-
Notifications
You must be signed in to change notification settings - Fork 271
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.
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
Stream SDK 6.0.3 sending a new message with image or file attachments is not storing the message or its attachments #4973
Comments
Hi, sorry for the late reply - we are looking into it. |
Hello @SchramboVida On the logs, we can see the attachment is properly uploaded to your CDN, and a new attachment with the URL Could you provide us with the following logs after the attachment is uploaded? If you could replicate this error in our Sample App would be awesome, because it would help us to debug+fix it. I have been doing some tests with our Sample App and it seems to be working fine.
|
Hi @JcMinarro, I've tried again using a stubbed-out uploader implementation as suggested which just returns success from the The failure occurs before any of the custom CDN uploader code executes, specifically in Before That's strange because the Interestingly, the failure result does not seem to propagate back to Look for the
Thanks again for your help and please let me know if you have any questions! Best Regards, |
Hello @SchramboVida Thanks for your detailed reply. I will need more time to debug it properly and try to find why it is failing in your side. |
Hi @SchramboVida, One thing concerns me in your logs:
attachmentsSender
.sendAttachments(preparedMessage, channelType, channelId, isRetrying, repositoryFacade)
.also { result ->
debugger.onInterceptionStop(result)
} If |
Based on the logs it seems like this line does't suspend jobsMap[newMessage.id]?.join() Which makes me to believe that |
@SchramboVida were you able to solve the problem? |
Hello. I have encountered a similar trouble and conducted my own investigation and findings, so I would like to report them. (Please note that English is not my native language, so my expressions might not sound completely natural. 🙇 ) There seem to be two main issues causing this behavior. Issue 1:
|
/** | |
* Idle state before attachment starts to upload. | |
*/ | |
public object Idle : UploadState() { override fun toString(): String = "Idle" } | |
/** | |
* State representing attachment upload progress. | |
*/ | |
public data class InProgress(val bytesUploaded: Long, val totalBytes: Long) : UploadState() |
Additionally, since jobsMap[newMessage.id]
does not consider any UploadState
other than Success
and Failed
, allAttachmentsUploaded
will always be false, resulting in waitForAttachmentsToBeSent(...)
always returning Result.Failure
.
Lines 145 to 155 in a5f43b8
attachments.all { it.uploadState == Attachment.UploadState.Success } -> { | |
messageToBeSent = repositoryFacade.selectMessage(newMessage.id) ?: newMessage.copy( | |
attachments = attachments.toMutableList(), | |
) | |
allAttachmentsUploaded = true | |
jobsMap[newMessage.id]?.cancel() | |
} | |
attachments.any { it.uploadState is Attachment.UploadState.Failed } -> { | |
jobsMap[newMessage.id]?.cancel() | |
} | |
else -> Unit |
Issue 2: Messaging failures are not reflected, making it appear as if they were successful
The default implementation of the MessageComposer
component does not include a callback for sendMessage(...)
call. (At the time of the reported in SDK 6.0.3, callback
were not designed to be passed in the first place.)
As a result, the Result.Failure
returned by waitForAttachmentsToBeSent(...)
is not handled, and the messaging process terminates without proper feedback.
Line 140 in a5f43b8
onSendMessage: (Message) -> Unit = { viewModel.sendMessage(it) }, |
Lines 176 to 179 in a5f43b8
public fun sendMessage( | |
message: Message, | |
callback: Call.Callback<Message> = Call.Callback { /* no-op */ }, | |
): Unit = messageComposerController.sendMessage(message, callback) |
Furthermore, even after the messaging process is completed, the UploadAttachmentsAndroidWorker
continues to run in the background and updates the message cache after upload attachments. So, though I haven't delved into it in detail, I suspect that the cached messages are giving the illusion of successful message sending until the app is re-launch.
Lines 91 to 92 in a5f43b8
val attachments = uploadAttachments(message) | |
updateMessages(message.copy(attachments = attachments)) |
Lines 154 to 156 in a5f43b8
// RepositoryFacade::insertMessage is implemented as upsert, therefore we need to delete the message first | |
messageRepository.deleteChannelMessage(updatedMessage) | |
messageRepository.insertMessage(updatedMessage) |
My Solution
To address this issue, I forked and customized the behavior of waitForAttachmentsToBeSent(...)
. Specifically, after executing enqueueAttachmentUpload(...)
, I ensured that the worker's completion is waited for synchronously, allowing allAttachmentsUploaded
to be evaluated correctly.
ref: main...10llip0p:stream-chat-android:patch-attachments-sender
However, this solution is merely a temporary workaround and not considered the definitive solution. I hope this issue will be resolved in a more comprehensive manner.
Thank you.
@JcMinarro @kanat @DanielNovak My apologies for this very late reply, I fell ill shortly after my initial report, then once recovered was busy implementing other app features. Now that those tasks have been completed, I've had some time to continue debugging and have a much better understanding of the cause as well as a solution for your review and additional feedback. Problem SummaryAt the core of the problem we have
The call to Here's our
When the user sends a message with one or more attachments, a While the attachments are uploading, Here is the relevant section of our
Note that the At this stage of uploading, the attachments have no
SolutionTo make
My ConcernsThe Can we trust that this internal Stream API and related dependencies on the Thanks! |
Hello @SchramboVida I just adapted our Sample App to use a custom Could you review this implementation and compare it with yours? |
This comment is very helpful for me. The problem summary is likely to be very similar to what I reported, but the solution is more appropriate and smart than my one.
I believe this is the essence of the problem. Without resolving it, even when implemented according to the documentation, it is not possible to upload files (even when using the default Stream CDN as I did). Line 28 in 0b26a3b
|
Ok. If the OfflinePlugin is not added during the configuration process, a Aren't you adding While we fix the issue when the
|
Hi @JcMinarro, thanks for confirming what I suspected, that The documentation states this plugin is optional, and since our app does not work offline we chose to not include it. However, including it resolves the problem without my Thanks again for the help! |
The idea is |
I found that my problem was caused for the same reason. And I was able to solve it by using the offline plugin too. |
BTW. |
Describe the bug
Using Stream SDK
io.getstream:stream-chat-android-compose:6.0.3
(we're upgrading from 5.17.10) we have theChatClient
configured to use a custom CDN uploader via.fileUploader(ChatFileUploader())
.Our implemention of
ChatFileUploader()
which is a subclass ofio.getstream.chat.android.client.uploader.FileUploader
provides the requiredsendImage(...)
andsendFile(...)
implementations and returns ags://
schemed url that identifies the asset for subsequent access, for examplegs://vida-stream-attachments-test/7098a6a7-c47d-4d4c-b24e-9d6f7c8d8e7f/8e118e6f-48e8-4812-b8c0-74bcbf765ff7.quicktime
We have custom image attachment composers that use the
gs://
urls to obtain an authorization token that's needed to pull the actual image asset out of our CDN for viewing. All of this has been working perfectly up to SDK 5.17.10 but is not working at all in SDK 6.0.3 and we're a bit stumped why.SDK version
To Reproduce
Steps to reproduce the behavior:
ChatClient
with a custom uploader that returns a url-like string for the asset, e.g.io.getstream.result.Result.Success(UploadedImage(file = "gs://vida-stream-attachments-test/7098a6a7-c47d-4d4c-b24e-9d6f7c8d8e7f/8e118e6f-48e8-4812-b8c0-74bcbf765ff7.quicktime"))
Expected behavior
The new message with freshly uploaded attachment should persist in the channel message list. The message should remain loadable via
chatClient.getMessage(messageId)
and not return HTTP 400 not-found errors "Message with id *** doesn't exist".One thing that's a bit odd is the error string — when I search the SDK 6.0.3 sources for the error string, the closest I can find is in
io.getstream.chat.android.client.attachment.worker.UploadAttachmentsWorker.uploadAttachmentsForMessage(String)
but that does not have the "doesn't" contraction visible in the error screen below, so I'm not sure this is actually the source of the error string.Device:
Screenshots
Debug log output (with comments)
The text was updated successfully, but these errors were encountered: