-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
SocketTimeoutException from fetching number of images deleted #1876
Comments
Ugh, I just checked our Google Play stats and this appears to be a major cause of crashes - 60+ reports in the last 7 days. Unfortunately it seems that if we don't have time to find an ideal solution, we will have to remove the quiz checker in 2.8.3 and reinstate it once an ideal solution is found. Full stacktrace: Report 1
|
How catching the exception and such cases considering that the quiz is not needed (which is the less intrusive option)? |
@nicolas-raoul The main problem with doing that is, what do we return as the value of |
I created a PR. If anyone wants to take this up and fix the crash properly (please bear in mind that you cannot just ignore the timeout or return null), please feel free. We can re-release the quiz as soon as it is fixed. |
@misaochan I have read the comments and your PR but, couldn't understand why we cannot just catch the exception and cancel quiz if caught. Can you please explain in more detail? |
@misaochan I had the same doubt as @neslihanturan. Why cant we just handle the exception and delegate the flow accordingly, ie. log the exception for reference purposes and maybe set isRevertCountFetched=false [Just to stay safe] ensuring that callQuiz() is never called. Is there something I am missing ? |
I have added a PR refactoring some of the error-prone code pieces. Will take a final look at these modules tomorrow and makes any changes if necessary. The basic flow works well. PS: Personally I was feeling bad about reverting some of @tanvidadu's changes so I decided to pitch in. :) |
@neslihanturan @maskaravivek @neslihanturan My main concern is that we really need to get this fix out ASAP (i.e. by Thursday) due to the 12% crash rate for users. I didn't have the time to spend a few hours going over the code, and I didn't want to pressure anyone else to work overtime to fix it. ;) However, if you guys are willing to put in the time to ensure that this fix is done (and thoroughly tested) so that we can release 2.8.3 by Thursday at the latest, we can definitely push the proper fix instead of the stopgap measure. Unfortunately I will be travelling most of next week, and unable to push any fixes then. We need to give it a bit of a buffer because we need people to test the beta before we can push to prod. |
Hmm, my Achievements are not working after #1881 was merged. I am always Level 1, and images uploaded are 0/20 despite them registering correctly on ContributionsActivity's action bar. This happens for both misaochan2 and misaochan, logging out and in again does not fix it. @maskaravivek do they work for you? |
Everything should hopefully be sorted now, so will close this and reopen if needed. Thanks for volunteering to fix it @maskaravivek . :) |
Summary:
I logged in to Misaochan, went to Nearby, and the app crashed for me while I was searching for an image to upload to Nearby. Stack trace seems to point to getRevertRespObjectSingle() in ApacheHttpClientMediaWikiApi
Steps to reproduce:
Unsure of how to reproduce, it is not consistent. However, we should handle this exception more gracefully. I think catching it and outputting an error message is sufficient since that method is not essential (it's just used to get user revert rate). [Edit: After looking into it further, this is NOT true - this function is called whenever the user loads ContributionsActivity, and it is nonNull.]
Add System logs:
Device and Android version:
Android 8.0 Samsung Galaxy s7
Commons app version:
#1875 which is based on
2.8-release
, but the error does not appear to be relevant to that PR at all, just coincidenceWould you like to work on the issue?
Pref not
The text was updated successfully, but these errors were encountered: