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

Upload failures can result in an inability to retry uploads #6583

Open
dcalhoun opened this issue Jan 26, 2024 · 11 comments
Open

Upload failures can result in an inability to retry uploads #6583

dcalhoun opened this issue Jan 26, 2024 · 11 comments
Assignees
Labels
Media [Type] Bug Something isn't working

Comments

@dcalhoun
Copy link
Member

As reported in p1706285247418489-slack-C063XEQAM8X, it is possible to arrive in a state where one is unable to retry failed upload.

Screen recording
retry-upload-issue.mp4

Steps to Reproduce

  1. Insert a media block.
  2. Induce a failure (likely one other than completly disconnecting the network connection).

Expected Result

Tapping the failed media upload and then "Retry" results in the upload restarting and the UI showing upload progress.

Expected Result

Tapping the failed media upload and then "Retry" results in no changes to the UI and presumably not upload restarting.

@dcalhoun dcalhoun added [Type] Bug Something isn't working Media labels Jan 26, 2024
@derekblank
Copy link
Contributor

derekblank commented Jan 29, 2024

I attempted debugging this today and could not fully replicate the issue.

It is difficult to replicate a media upload failure to produce the scenario described in the video above. I was eventually able to produce a failure by following these steps:

  1. Turn off WiFi and throttle the mobile device connection to 3G or or 4G
  2. Upload media (ideally, lots of media)
  3. Wait for a media upload to fail

After producing a failed state for an uploaded media, I was able to tap on the media and hit "retry", and the media would resume uploading. To me, this suggests that the retry functionality is not broken, but there may be another circumstance where the retry option is available and the media will not upload. For example, when the device is fully offline (no WiFi, and no mobile connection), the "Retry" option is still available in the ActionSheet. Pressing it will do nothing, of course, as the device is offline. However, in my testing, the offline indicator was displayed in the Editor to tell the user this, but it is not present in the video reported by @fluiddot above.

Here is a video of my experience using the 24.1 WPiOS release build in pr22489-12d4aa4:

  • The video starts with the device offline and several failed media uploads.
  • [0:02] I tap on a failed media upload and hit Retry, and nothing happens as the device is offline.
  • [0:08] I turn on mobile connectivity and a 4G connection is established. When I press "Retry", the status changes from "Failed" to "Waiting for Connection".
  • [0:21] I turn on WiFi.
  • [0:43] I press "Retry" on "Waiting for Connection" media and the uploads resume and eventually all succeed.
Example Video
Retry.Failed.Uploads.mov

@fluiddot Perhaps you could share more info on how you produced these media upload failures? Was it as simple as using a 4G connection? Were there any additional server response messages that may provide more clues (by tapping on the failed uploads)?

@dcalhoun It does appear there are still some edge case circumstances where paused/failed/retry status may be unclear to the user. For example, when the device is fully offline and the "Waiting for Connection" notice appears, it's possible to tap on the paused media and be presented with a "Retry" option in the ActionSheet. It's not at all as serious as a crash or data loss, but still could be somewhat confusing to the user. I would imagine that this option still being present in the ActionSheet is likely related to the media status being marked as "failed" when the device is offline.

In my testing, I was unable to replicate a state where the retry would not restart unless the device was completely offline and the "Working Offline" indicator was showing.

@dcalhoun
Copy link
Member Author

To me, this suggests that the retry functionality is not broken, but there may be another circumstance where the retry option is available and the media will not upload. For example, when the device is fully offline (no WiFi, and no mobile connection), the "Retry" option is still available in the ActionSheet. Pressing it will do nothing, of course, as the device is offline. However, in my testing, the offline indicator was displayed in the Editor to tell the user this, but it is not present in the video reported by @fluiddot above.

Another scenario where tapping "Retry" may have no impact is if the upload actually succeeded, but the UI declares otherwise. I wonder if there were conflicting media upload events (thumbnail?) that resulted in the state. @fluiddot do you recall if, after achieving this state, the post HTML included local (file://) or remote (https://) URLs?

For example, when the device is fully offline and the "Waiting for Connection" notice appears, it's possible to tap on the paused media and be presented with a "Retry" option in the ActionSheet. It's not at all as serious as a crash or data loss, but still could be somewhat confusing to the user. I would imagine that this option still being present in the ActionSheet is likely related to the media status being marked as "failed" when the device is offline.

If an upload fails due to lack of network connection or other server errors, the "Retry" action is available. I am wary of removing it for the former scenario. I think it is good to always present the "Retry" option to the user for any error to allow them to retain control. It would be frustrating to have a valid network connection, but have the app tell you otherwise and give no option to retry the upload.

@dcalhoun
Copy link
Member Author

Related to my question as to whether the "failed" uploads had local or remote URLs in the post content, the retry logic only proceeds if the media model does not possess remote URLs.

@fluiddot
Copy link
Contributor

@fluiddot Perhaps you could share more info on how you produced these media upload failures?

@derekblank Sure, I'll try to debug the app and upload requests to get more details. Something I noticed (and captured in the video) is that the upload progress is at 100% right before the upload fails.

Was it as simple as using a 4G connection?

Yes, simply using a 4G connection but provided from a router. The device I'm using for testing is connected via WIFI and the 4G connection is the one that is provided by my Internet provider.

Were there any additional server response messages that may provide more clues (by tapping on the failed uploads)?

Unfortunately, the error message just says "Something went wrong. Please try again". I'll try to collect more information about the error.

Another scenario where tapping "Retry" may have no impact is if the upload actually succeeded, but the UI declares otherwise. I wonder if there were conflicting media upload events (thumbnail?) that resulted in the state. @fluiddot do you recall if, after achieving this state, the post HTML included local (file://) or remote (https://) URLs?

@dcalhoun I've performed another test and confirmed that the source of the failed images points to local (i.e. file://). Another thing I noticed is that if I switch to HTML mode after retrying the images that failed (while the block state is still displaying that the upload failed) the state changes and displays the uploads as complete. However, the upload didn't finish. Similarly, if I stop any uploads, the blocks also change to a weird state where the upload indicator is displayed but it's greyed out.

ios-upload-issue.mp4

@derekblank
Copy link
Contributor

Another thing I noticed is that if I switch to HTML mode after retrying the images that failed (while the block state is still displaying that the upload failed) the state changes and displays the uploads as complete.

I was able to replicate this too. when switching from Visual to HTML mode and back again, the Image opacity went to 1 and the progress bar disappeared. However, in my case, the UI changed back to an "in progress" state after about five or six seconds, where the opacity was dropped back to 0.3 and the progress bar was displayed. It was still possible to tap on the image and be presented with "Stop Upload" in the ActionSheet. I'll see if we can improve this so that the UI persists the "uploading" state when switching between Visual and HTML mode.

@derekblank
Copy link
Contributor

derekblank commented Jan 30, 2024

After debugging further, it seems that the UI state is not being updated correctly in MediaUploadProgress to handle the logic for displaying the "in progress" upload overlay and progress bar. From my testing, image upload behavior will behave as intended on a throttled connection, but the UI state of the Image block is not always updated correctly in the Editor to indicate if the image is actually uploaded or not. In other words, the image upload is handled correctly on the back end, but the success/failure UI to the user can be incorrect.

I believe that this is because we are handling several potential display states of the upload progress in the Editor: uploading, paused, failed, success, etc. Comparing the WP app behavior against other apps like Tumblr and DayOne, the image upload progress state is not reported so explicitly in the UI: an image is selected, displayed in the editor, and then presumably failures are handled at a more macro "post" level, rather than on individual media uploads. We may have an opportunity to simplify things by taking inspiration from the upload flows on these apps. A "bigger picture" task would be to explore how these apps handle upload failures in the UI from the backend, but to break it into smaller chunks of work we could focus on the front end UI state in the WP apps for now.

The logic to handle the upload UI display state is managed by a handful of events: uploadState, isUploadInProgress, isUploadFailed, etc. These events control the display of the progress bar, retry message, and upload overlay opacity. I'll continue debugging to see how we can do better to prevent the UI state from getting out of sync.

Perhaps a broader solution is to simplify things and leverage the file:/// and https:// prefixes of the URLs to indicate upload success. Some of these ideas are being explored in WordPress/gutenberg#57869 where the local/network state is being used to transition the image more smoothly. In another comment, @dcalhoun suggested we could extrapolate this work into a hook that returns the prefetched URL to indicate upload state, e.g.:

const { error, isUploading, url } = usePrefetchedUrls(url);

We could assume that any file:/// URI would still be considered isUploading from a UI perspective, and any https:// URI could be considered successful. This may help clarify the UI sync issue described in this issue of when the image appears to be successfully uploaded in the UI, but it is not (and vice versa). If we chose this approach, I think it would likely require us to land the work in WordPress/gutenberg#57869 first and implement the proposed usePrefetchedUrls hook. Then, we could revisit this PR and apply the changes.

@dcalhoun / @fluiddot I'd be interested in your feedback on this approach. I haven't yet identified any quick wins on how to solve the UI sync issue on a very throttled connection, so we may need to look at a bigger picture solution. I will continue debugging on a quick win solution for the time being, but let me know if you note anything I've missed, and any further areas of focus that might be worth exploring.

@dcalhoun
Copy link
Member Author

Thank you for continuing to debug this!

I was able to replicate this [inaccurate visual states] too. when switching from Visual to HTML mode and back again, the Image opacity went to 1 and the progress bar disappeared.

Interesting. I suppose this oddity is a result of the upload state being communicated via a host event and stored in local component state. When the editor toggles from Visual to HTML to Visual, all the local component state is discarded and the editor presents a default visual state for image uploads. Possibly an edge case not worth prioritizing now.

The logic to handle the upload UI display state is managed by a handful of events: uploadState, isUploadInProgress, isUploadFailed, etc.

At some point, I think it is worth refactoring away from independent isUploadInProgress and isUploadFailed React component states and leverage a single uploadState as this would prevent the possibility of two conflicting states being active at once.

Perhaps a broader solution is to simplify things and leverage the file:/// and https:// prefixes of the URLs to indicate upload success.

Possibly. However, a successful upload will change the URL, but the URL will not change on failure. Thus, we may still need explicit media upload events from the host; namely, for error and paused events.

I'd be interested in your feedback on this approach. I haven't yet identified any quick wins on how to solve the UI sync issue on a very throttled connection, so we may need to look at a bigger picture solution.

It is still not clear to me why @fluiddot experienced the outcome in his original report and screen recording. Have we identified reproduction steps for that specifically or observed the arrival of conflicting upload state events?

@fluiddot
Copy link
Contributor

fluiddot commented Jan 30, 2024

We may have an opportunity to simplify things by taking inspiration from the upload flows on these apps. A "bigger picture" task would be to explore how these apps handle upload failures in the UI from the backend, but to break it into smaller chunks of work we could focus on the front end UI state in the WP apps for now.

I also tried the other apps and also observed simpler upload flows. I agree the JP/WP apps would benefit from simplification and would help reduce the confusion among users.

It is still not clear to me why @fluiddot experienced the outcome in his original report and screen recording. Have we identified reproduction steps for that specifically or observed the arrival of conflicting upload state events?

I'm testing today using a local build and I'm struggling to reproduce it again. Surprisingly, I could easily reproduce it yesterday and I don't think my network conditions have changed. The only difference is that I'm using the latest changes from trunk. I'll try tomorrow again, but if we don't manage to reproduce it again we could consider de-prioritizing the issue.

UPDATE: My previous tests were performed using the installable build from wordpress-mobile/WordPress-iOS#22482, as part of testing the release 1.111.2 (reference).

@derekblank
Copy link
Contributor

derekblank commented Jan 31, 2024

Perhaps a broader solution is to simplify things and leverage the file:/// and https:// prefixes of the URLs to indicate upload success.

Possibly. However, a successful upload will change the URL, but the URL will not change on failure. Thus, we may still need explicit media upload events from the host; namely, for error and paused events.

I agree. I'll hold off on exploring any updates to https:// and file:/// further for now.

It is still not clear to me why @fluiddot experienced the outcome in his original report and screen recording. Have we identified reproduction steps for that specifically or observed the arrival of conflicting upload state events?

In my testing, I have not been able to reproduce a state where the uploads will not retry when expected to (manually or automatically). The only reproduceable issue I note is the low-priority one where there is a delay in updating the UI state back to "in progress" when switching from HTML to Visual mode described here.

@fluiddot If you're able to note any reproduceable steps for your original issue, let me know and I am happy to continue investigating.

@fluiddot
Copy link
Contributor

fluiddot commented Jan 31, 2024

@fluiddot If you're able to note any reproduceable steps for your original issue, let me know and I am happy to continue investigating.

@derekblank I managed to reproduce it again by throttling the connection and forcing requests to timeout. Here's a screenshot of the profile I'm using:

Steps to reproduce it:

  1. Enable the above profile in the Network Link Conditioner.
  2. Open the app.
  3. Create a post.
  4. Add a Gallery block.
  5. Insert multiple images.
  6. Observe that the upload indicator doesn't progress.
  7. Wait until uploads fail.
  8. Observe the error message.
  9. Tap on "Failed to insert media. Tap for more info.".
  10. Tap on "Retry" option.
  11. Observe that the error message remains and no change is made to the block's state.
  12. Disable the Network Link Conditioner.
  13. Observe that after a couple of seconds, uploads are resumed.

I think the above scenario is similar to what I was experiencing when using 4G. However, I'm still unsure what was causing the timeout issue on the first tests I performed.

ios-upload-issue.mov

NOTE: The timeout error happened after one minute, so I've cut the video capture to make it shorter.

@derekblank
Copy link
Contributor

Thanks for sharing the replication steps. 🙇 I'll continue debugging using this flow and report my findings.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Media [Type] Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants