Skip to content
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

Tw 1702/hotfix error handling on download #1814

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

Te-Z
Copy link
Contributor

@Te-Z Te-Z commented May 28, 2024

Ticket: #1702

NEED MERGE FIRST: linagora/matrix-dart-sdk#61

Done:

  • fixed regression on error handling when downloading a file
  • refactored a bit the code for better testability
  • created tests for mobile
  • created tests for web
screen-20240528-190457.mp4

Error handling when there is no connection to the server:

downloadError.webm

Copy link

This PR has been deployed to https://linagora.github.io/twake-on-matrix/1814

@Te-Z Te-Z marked this pull request as draft May 28, 2024 17:42
@Te-Z Te-Z marked this pull request as ready for review May 28, 2024 18:48
@sherlockvn
Copy link
Contributor

sherlockvn commented Jun 4, 2024

I tested in IOS, its only show the error in 1/3 cases. wdyt @Te-Z

RPReplay_Final1717490331.2.MP4

@Te-Z
Copy link
Contributor Author

Te-Z commented Jun 6, 2024

I tested in IOS, its only show the error in 1/3 cases. wdyt @Te-Z

RPReplay_Final1717490331.2.MP4

Hi @sherlockvn this a problem indeed. I can't reproduce it on my side since I don't have a macbook. But looks like it works on web and android. Can you help me debug it please ? If not maybe we will create a ticket to debug it on iOS

@sherlockvn
Copy link
Contributor

I tested in IOS, its only show the error in 1/3 cases. wdyt @Te-Z
RPReplay_Final1717490331.2.MP4

Hi @sherlockvn this a problem indeed. I can't reproduce it on my side since I don't have a macbook. But looks like it works on web and android. Can you help me debug it please ? If not maybe we will create a ticket to debug it on iOS

i will create another ticket, because im in another ticket right now

@Te-Z Te-Z force-pushed the TW-1702/hotfix_error_handling_on_download branch from 2988570 to b9c3544 Compare July 9, 2024 15:14
@Te-Z
Copy link
Contributor Author

Te-Z commented Jul 9, 2024

I tested in IOS, its only show the error in 1/3 cases. wdyt @Te-Z

RPReplay_Final1717490331.2.MP4

@sherlockvn looks like it's fixed after rebase. I'm not able to reproduce it. Could you try again ?

@Te-Z
Copy link
Contributor Author

Te-Z commented Jul 11, 2024

@sherlockvn actually what you have is the normal behavior of download manager: if connection is lost and not error is thrown, the file stays in the queue to be able to continue download

@sherlockvn
Copy link
Contributor

sherlockvn commented Jul 22, 2024

@sherlockvn actually what you have is the normal behavior of download manager: if connection is lost and not error is thrown, the file stays in the queue to be able to continue download

it will not continue downloading.

RPReplay_Final1721622320.MP4

@Te-Z
Copy link
Contributor Author

Te-Z commented Aug 19, 2024

@KhaledNjim could you please provide a demo of the fix you've done ?

@KhaledNjim
Copy link
Contributor

@KhaledNjim could you please provide a demo of the fix you've done ?

Added in the pr description

@Te-Z
Copy link
Contributor Author

Te-Z commented Aug 19, 2024

@sherlockvn it should be good now. Could you review it again please ?

@sherlockvn
Copy link
Contributor

i tested and i worked well in iOS.

@sherlockvn
Copy link
Contributor

please rebase

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants