OpenArchive / Save-app-android-old

This is the Save app for Android
https://open-archive.org
GNU General Public License v3.0
94 stars 26 forks source link

Code error language is displayed when session is expired #537

Closed purvi-ranawat closed 4 months ago

purvi-ranawat commented 5 months ago

Describe the bug

When session expires, code error is displayed instead of the proper error message.

Below is the screenshot :

image

Environment (please complete the following information): OS version: Android 14

tladesignz commented 4 months ago

@uniqx, I just had a similar request for iOS:

https://github.com/OpenArchive/Save-app-ios/issues/276

The only sensible improvement in my eyes, is to extract the important information from the data structure and display only that.

I did this in a somewhat generic manner for the Dropbox error messages on iOS: https://github.com/OpenArchive/Save-app-ios/commit/b4cb8119db593604da01fc91d2473e66fc1f3b29

Annoying hurdles: the Dropbox lib uses its own JSON deserialization in a super-weird manner. Hence the wild code.

I hope for you, it's easier on Android. ;-)

Let me know if you need assistance.

ryjen commented 4 months ago

I hope they are localized. I think long term it makes more sense for the Save app to have its own error messages for that reason. They can be generic for all and use analytics for troubleshooting. What do you think? @tladesign

ryjen commented 4 months ago

I have a PR here, please let me know if you have comments.

@losalim @foundscapes Will need a string / translation for:

upload_unsuccessful_description = Unable to upload due to session error, please try again or contact support.

tladesignz commented 4 months ago

I hope they are localized.

No, of course these error messages are not localized. They are API errors.

I think long term it makes more sense for the Save app to have its own error messages for that reason. They can be generic for all and use analytics for troubleshooting. What do you think?

Regarding analytics, see this comment: https://github.com/OpenArchive/Save-app-android/issues/491#issuecomment-1964030647

Regarding generic error messages, I thought I already commented on that in the sibling issue on iOS: https://github.com/OpenArchive/Save-app-android/issues/491#issuecomment-1964030647

In my eyes, you're not improving anything. You're just replacing slightly weird-to-look-at JSON, which might make no sense to the average user, with unhelpful generic corporate speak void of almost any helpful content, which also doesn't help the user.

To dissect the suggestion from your Pull Request:

These are the information pieces:

Unable to upload

Ok, well. The user already figured that out, probably from the current title of the alert box and the red exclamation mark.

due to session error

Does the average Joe better understand what a "session error" is instead of "expired_access_token"?

please try again

That seems the only helpful advice. Maybe it's good to repeat this in the alert box text. But it's also already an action suggestion in the alert buttons.

or contact support.

Which support? How to contact it? We don't have any. This is actively misleading. A smoke candle.

Of course I understand, that JSON looks ugly. Many users will be annoyed seeing that. But they will also be annoyed seeing generic unhelpful error messages. The raw stuff at least gives some users some helpful indications of what might have gone wrong.

Happy to beautify some of that (e.g. stripping the useless content of the JSON) for certain, as generic-as-possible situations like the errors from the Dropbox library. But for maintainability reasons and to balance out gain and investment, not happy to try to catch every single case of what could go wrong.

There's multiple layers of sources of errors:

Good luck handling all of these properly, finding them, reproducing them for testing. We can sink a lot of time and money into this. And we can also make the product worse with generic solutions.

ryjen commented 4 months ago

Most people do not care what the error is, only how to resolve it.

If all of the dropbox errors end up being resolved with "log in again or re-add the backend" that's not really great, but its the same thing for a user.

Ideally @uniqx you would have a better solution for this than mine. I will have to go ahead and find a better string or figure out the string owner.

I do not profess to be a UX specialist; the suggested verbage is merely what I would expect in a different environment. Calling support would not be feasible for this app's user base. I am not sure how technical they are either if they are Tor enabled, and JSON would be fine as it is for me.

As you said, handling every single error is ridiculously complex from the dimensions of localization and testing. Errors bubble up from the data layer, and the UI makes it friendly with a big mask like all the other data, guiding the user as best it can.

tladesignz commented 4 months ago

Most people do not care what the error is, only how to resolve it.

True. But to get to an understanding of how to resolve the error, one needs to know, what the error actually is.

Of course, we could go the long way and do all the necessary deduction from error to resolution proposal in code.

However, that leads us to the same problem again: There's hundreds of different errors from the different layers. We cannot possibly identify even most of them and provide that logic.

Hence, a second best solution is to let the users know, what the error is, so they can make a more-or-less educated guess what to do about it.

Hiding the error and just showing generic nonsense seems to be the third best option.

If all of the dropbox errors end up being resolved with "log in again or re-add the backend" that's not really great, but its the same thing for a user.

Actually, there not. There's multiple different errors classes that can happen:

I do not profess to be a UX specialist; the suggested verbage is merely what I would expect in a different environment. Calling support would not be feasible for this app's user base. I am not sure how technical they are either if they are Tor enabled, and JSON would be fine as it is for me.

My guess is, in terms of technical capability we have a very mixed bag. There's probably users who are very savvy who can even make something out of raw JSON data. On the other spectrum, there are people like Natalie who run away screaming as soon as they see anything which they don't expect and looks too technical. And there are people who never read anything anyway, which stands in the way to what they want to achieve.

We cannot help the latter in any case. In networking, there's always a high risk of something going wrong, and we have to communicate that in some way. People who choose to ignore that, cannot be helped, besides giving them the most important option on hand ("retry").

It's hard to deal with the second category. To make them comfortable, the effort increases enormously.

However, we should comfort the people somewhere in the category between 1 and 2, by cleaning up errors as best as we can in the most generic way we can. Those who are confused by too many brackets but can make something out of a low-level error message at all, should be helped.

Maybe we want to go some extra mile for certain edge cases like a full disk on the target system. ("Go, clean up your Dropbox!")

ryjen commented 4 months ago

Unfortunately, all I can say is that these kinds of things (error handling and states) have to be at least attempted to be flushed out visually in a design phase. It would help stakeholders have something to expect, and implementors to know what to do.

tladesignz commented 4 months ago

Yeah, sure. You can complain about all the failures which happened before you. Or you can just try to improve things around you.

foundscapes commented 4 months ago

plan: meeting w/ ux designer + @ryjen + me to resolve

ryjen commented 4 months ago

This has bigger scope than the error message. We can add a string but it needs to be localized and will probably change again anyway.

Ideally, refreshing the session should happen without the users knowing.

See #575