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

Prevent retries for genuinely failed uploads #5284

Open
RitikaPahwa4444 opened this issue Sep 5, 2023 · 10 comments · May be fixed by #5613
Open

Prevent retries for genuinely failed uploads #5284

RitikaPahwa4444 opened this issue Sep 5, 2023 · 10 comments · May be fixed by #5613

Comments

@RitikaPahwa4444
Copy link
Collaborator

What is the user problem or growth opportunity you want to see solved?

No response

How do you know that this problem exists today? Why is this important?

The app retries all failed uploads, even if they fail for a genuine reason like invalid filename. This impacts the UX badly [details].

Who will benefit from it?

No response

Anything else you would like to add?

No response

Glassar added a commit to DD2480-group8-VT24/apps-android-commons that referenced this issue Feb 27, 2024
Glassar added a commit to DD2480-group8-VT24/apps-android-commons that referenced this issue Feb 27, 2024
Glassar added a commit to DD2480-group8-VT24/apps-android-commons that referenced this issue Feb 27, 2024
@Glassar
Copy link

Glassar commented Feb 27, 2024

Hi, me and my group are taking a crack at this as part of a uni course for the next week. Could we get assigned this?

@nicolas-raoul
Copy link
Member

@Glassar It is yours! :-) If too hard, easier issues like #5565 or #4895 are available.

(note for GSoC: If this bug gets solved, we will replace the GSoC's tasks with another of similar size)

@Glassar
Copy link

Glassar commented Feb 28, 2024

Thank you, and great to have those suggestions. We have done a first code inspection and think this should be doable though so we'll start here

@Glassar
Copy link

Glassar commented Feb 29, 2024

We have done quite a bit of digging and found where an upload attempt fails (in uploadWorker, or this is at least where the failure is determined).

So our plan right now is to add code to the "failure" points in UploadWorker.uploadContribution which passes the failure reason onwards. This can then be used in ContributionsFragment.retryUpload to determine if this is a genuine failure and the attempt abandoned, or if it should try again. Finally this failure reason could also be used give the user better information about why their upload has failed repeatedly link.

But to determine what is a genuine failure, we need to know/decide what is a genuine failure which can the be used as a whitelist to stop retrying (one obvious example which won't change is if we get a FileNotFoundException). And this might be identified through for example exception messages, but we are having some trouble with determining what those exact conditions might be. Any ideas of the kind of errors might belong here, or where we should look?

Another thing we are wondering about is passing the information. The easiest and most intuitive way would be to add another field to the Contribution class for an error message as this how if it failed is already passed around. But the contributions are stored on the DB and as such we are not sure it storing the failure information there is optimal, any tips here?

Finally it seems reasonable to notify the user about what is going wrong with their upload, both when it fails to many retries and when we determine that it is a genuine failure. In what way should these notifications be sent (as far as I can see there is already sent something to the user in UploadWorker when it fails)?

@nicolas-raoul
Copy link
Member

One case I sometimes see is when the picture has been uploaded already, but the app somehow believes it failed, and retry.

@RitikaPahwa4444
Copy link
Collaborator Author

RitikaPahwa4444 commented Feb 29, 2024

I shared one reason for the case Nicolas mentioned above in this issue: #5280, might help.

@RitikaPahwa4444
Copy link
Collaborator Author

Any ideas of the kind of errors might belong here, or where we should look?

Invalid filenames also cause this issue. I tried adding a § sign in the caption, it keeps failing.

@Glassar
Copy link

Glassar commented Feb 29, 2024

Any ideas of the kind of errors might belong here, or where we should look?

Invalid filenames also cause this issue. I tried adding a § sign in the caption, it keeps failing.

Thank you, its super useful to have a way to reproduce this issue. Do you know of any other patterns that causes it to fail?

@ShashwatKedia
Copy link
Contributor

ShashwatKedia commented Mar 1, 2024

Invalid filenames also cause this issue. I tried adding a § sign in the caption, it keeps failing.

Hey, I was just trying to reproduce this issue, but surprisingly for me, the image successfully uploaded with a § in the caption:

Testing invalid filename

@MertD95
Copy link

MertD95 commented Mar 8, 2024

We have gotten quite far on the way to wrapping this up and it should techinically be able to be roled our (more details in the PR), but we are unable to continue due to our course running out.

The things left to implement is a more complete whitelist over "genuine" failures as we have not been able to actually reproduce any examples of it. Also it would be good if the user was notified about said "genuine" failures and so they can address it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment