-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Sometimes uploads fails or queued, then don't upload. #1878
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
Comments
Where did John post, @neslihanturan ? I'd like to ask him about the method he used to upload (Nearby? Share? etc) |
I asked same question too, will update once the answer comes. He asked from twitter, I don't think he has a github account. |
He said contributions were from main screen. |
Could you point us to the conversation on Twitter please? |
Do you mean, seeing screenshots from conversation? |
Sorry, I'm not familiar with Twitter and didn't know you could send private messages on it - I thought he tweeted it, haha. If it's a private message then never mind. Could you ask him how often it happens for him, and if the queued pictures ever succeeded? Also, he has confirmed he is on 2.8.2? |
|
Yann posted in beta feedback:
Do you think the changes that we made to file uploads in 2.8 might be the culprit? @neslihanturan could you please ask John if he has a rough idea of when the problem started for him? |
Wanted to bump this issue as we have another report on google groups:
|
Yann has informed me that v2.8.4 does not solve this problem for him. I have been talking to Jim Henderson, who reports a similar issue with queued uploads. @neslihanturan , after you are done with your main UI PR, could you please go through your old PR with the file upload changes, and see if anything that you changed could have caused this? I suggest you try with an Android 8.0 or 8.1 emulator. So far all of the reports appear to be from that version. I just need to confirm with Jim about his Android version |
FYI my Android version is 7.1.1, and it is the latest update on my J5. Thanks for trying to fix this, it is rather annoying. It seems to happen specifically when I upload the first picture in a series. |
@Yannf Thanks for chiming in! Could you please elaborate on what you mean by "first picture in a series", by detailing the steps you take and the results? E.g.
So on and so forth? |
If I upload several pictures successively, usually only the first fails. And it seems the App crashes (it closes without warning). |
@ashishkumar468 Could you please work with @neslihanturan to solve this? It is our most urgent bug at the moment, so we will need this to be fixed in v2.9. You guys can contact @Yannf here on this thread, or Jim at Commons app talk , or others at our google group forums, if you need further information. |
@misaochan Will take this up |
I was actually trying to debug this issue on Google Pixel. Seems like something is wrong with the api's. which states that it was expecting png while we sent the file as jpeg. I tried hard coding the image extension to png and the upload succeeded [Shocking]. I could add an exception where in I could check if the photos are uploaded from google photos make them png by default. Does not seems right but would fix the upload fail cases from google photos. Also, from the code i can conclude that this should be happening since quite a while as say the comments as well, TODOing to add fix for uploads via Google Photos,. @misaochan @neslihanturan What do you guys suggest ? |
Actually not a backend bug, Although in the image info, Google Photo shows that the image is png but while sharing it, in the intent it shows that the image is jpeg and hence our backend rejects that because of which the upload fails |
@ashishkumar468 thanks for digging it! I think you are very close since we recently implemented some changes about file extensions. We convert some extensions to .jpeg or .jpg (I am not really sure but can check). |
@neslihanturan Yes, we convert jpeg to jpg but that is not causing any trouble. The issue is that google photos app is giving the mime type for png images as jpeg which is causing the api to fail. |
Hi,
On Tue, 9 Oct 2018, 00:29 Ashish Kumar, ***@***.***> wrote:
@neslihanturan <https://github.com/neslihanturan> Yes, we convert jpeg to
jpg but that is not causing any trouble. The issue is that google photos
app is giving the mime type for png images as jpeg which is causing the api
to fail.
But it only happens sometimes, and it seems in specific circumstances.
Yann Forget
… |
@Yannf Yes and that is what is causing the api's to fail at least in my case. AFAIK, upload does not fail always, right ? |
No, not always.
I checked my software. I am using the default Gallery application version
5.4.03.2 on Android, in case it matters.
Yann Forget
…On Tue, 9 Oct 2018, 00:36 Ashish Kumar, ***@***.***> wrote:
@Yannf <https://github.com/Yannf> Yes and that is what is causing the
api's to fail at least in my case. AFAIK, upload does not fail always,
right ?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#1878 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AFfmLT7K9jfqibJKUdJ30HuBcJgYLJQCks5ui6IwgaJpZM4WY5aT>
.
|
@Yannf and the uploads are failing in that as well ?. It would be great if you could attach the logs ? |
@ashishkumar468 Interesting, I had no idea that .png photos were so common, I was under the impression that most phone cameras take photos in .jpg! Indeed that is our failing, as we needed to quickly patch an urgent overwrite bug in production, therefore the upload service was simply made to expect a .jpg. You can see more information about that here - #228 (comment) and here - #1838 . We had intended to fix this when we implemented multiple uploads, but it seems that was a mistake. As I mentioned to @neslihanturan on that issue:
So if that is indeed the cause of this bug, then I guess the solution should be to formulate a proper solution for #228 that appropriately detects mimeType. Once the .jpg hack is removed, this should be solved. The TODOs to add fix for Google Photos are actually a different issue entirely - that was the issue of people experiencing crashes when uploading from Google Photos due to not having permission to read from that Uri. That has already been fixed I believe. @Yannf Would it be possible for you to check what extension your queued/failed photos have? Also, it is important to note that if the file extension was the issue, the failure/queued image should only happen if you upload via Share. It should not happen if you upload via the in-app Gallery button. So it would be great to see if you get different results by using the in-app Gallery button, then we can be sure that the #228 fix is the cause of the problem. :) |
@misaochan The main job would be to validate the file extension somehow locally. As I have mentioned earlier, even if the extensions are png google photos are giving the MIME type as jpeg. We will have to figure out a way to validate file extensions. Apart from this indefinite progress in notification failed uploads, happening on my device due to the same reason and also faced by Jim as well would be resolved if we are able to add a patch for this. |
Yes, we certainly need to check the file extension locally. I would be surprised if the error lies in the Google Photos API though, as I haven't been able to find any reports of that. I would have thought that it is our app erroneously assigning the MIME type as .jpeg, due to the hack I mentioned above. |
* Added a java library to fetch the MIME type from input stream * Fetch MIME type using this and use the contribution tag only when this fails:
@misaochan @neslihanturan I have added a fix for MIME type detection, although could not find a logical bug in the method which actually attaches the extension. Although the flow needs to be refactored a bit which we can take later on. Seems like this will add a fix for the image extension issue. Raising a PR for the same |
Had a basic question although , do the releases not go through master branch, as I could conclude from the thread that the issue went through the branch 2.8-release. If so, which branch am I supposed to make the PR to ? I had solved the issue on a branch cut from 2.8-release. Do I need to do it from master. It would be great if some one could clarify the same |
@ashishkumar468 The procedure for branching and releases is detailed here: https://github.com/commons-app/apps-android-commons/wiki/Project-maintenance . It is probably a good idea to read that entire page of our wiki actually, as part of your onboarding to our team. ;) So, the answer to your question is: It depends on whether we should release a hotfix (i.e. v2.8.5) for your fix (in which case it should go to 2.8-release) or whether we should just include it in v2.9 (in which case it should go to master). I think keeping it in 2.8-release is a good idea since your fix was so quick, so if it works then I can release it to v2.8.5 without having to wait for the other items needed for v2.9 (see #1830 ). We can cherry-pick it to master afterwards. An additional hint: We usually use Google Hangouts for day-to-day team communication that isn't particularly useful to record in the issue itself like "Which branch should I submit this to?" (in contrast, it is correct to discuss the workflow for the fix here on GitHub, as you did). I have sent an invitation to you a couple of days ago. :) |
* Bug Fix #1878 * Added a java library to fetch the MIME type from input stream * Fetch MIME type using this and use the contribution tag only when this fails: * formatting changes, removed unused commented out line
* Bug Fix commons-app#1878 * Added a java library to fetch the MIME type from input stream * Fetch MIME type using this and use the contribution tag only when this fails: * formatting changes, removed unused commented out line
Summary:
Our friend John Lubbock says, "I wonder if you know why when I upload photos on the Commons app, sometimes they say 'Failed' or 'queued', and don't upload?".
Device and Android version:
Samsung S9+
Commons app version:
2.8.2
Screen-shots:

Would you like to work on the issue?
^_^ no.
The text was updated successfully, but these errors were encountered: