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

AwfulErrors are unhelpful #688

Open
baka-kaba opened this issue Apr 14, 2019 · 2 comments
Open

AwfulErrors are unhelpful #688

baka-kaba opened this issue Apr 14, 2019 · 2 comments

Comments

@baka-kaba
Copy link
Contributor

baka-kaba commented Apr 14, 2019

The idea is these are meant to spot problems with successful responses from the site, like error messages etc, and then provide that information through getMessage. AwfulFragments etc show a popup with that information when a request results in an error (either an AwfulError or a normal VolleyError for general network stuff), so the user knows something went wrong and what it is. Ideally the info scraped from an error page is what the user sees in the popup, so they get the same info they would on the desktop site

The trouble is there's a limited set of AwfulErrors, so most problems will be classed as ERROR_GENERIC_FAILURE. This is the only one that specifies an error submessage, which also shows up in the popup, but the message is "check your network connection and try again" - which is rarely the issue, and if we're parsing a completed response with checkPageErrors then the network should be fine on the user's end anyway

And AwfulRequest has an overridable customizeProgressListenerError callback that rewrites the main error message (but not the submessage). Which would be fine for most things - a specific request can create a specific failure message - but it can replace important information. Like there's a problem with VoteRequest right now - here's what the error starts as

Specified thread was not found in the live forums.
Check your network connection and try again

and it ends up being displayed as

An error occurred while voting, please try again
Check your network connection and try again

which is completely misleading and throws away the useful error message that would tell people that actually it's not a network problem and retrying probably won't work because something's broken. And it would help us debug more easily too


Anyhow, I think we need

  • error messages should be general (basic description of the type of error), submessages should be specific (any error data we identified, messages scraped etc)
  • some of the other AwfulError types might be able to use submessages too, so check if there's anything useful that could be added there
  • maybe we could expand the set of AwfulErrors instead of "logged out, forums closed, access denied, generic unspecified error for everything else``. Spotting Cloudflare error pages would definitely be good
  • customizeProgressListenerError should be able to rewrite the main message and the submessage (including removing the latter), so it can fully customise the error popup. This way we have the option of a "clean" popup with no error data, but the customiser has to explicitly clear the data section of the error, so it won't happen by accident when you just want to tweak the basic error message the user sees
  • I'm still not sure ERROR_PROBATION should be a thing - it's not an error, it's just an attribute, like "you have an unread PM". Being on probation can cause errors, like trying to reply to a thread, so maybe that should be an error, ERROR_POSTING_ON_PROBATION or something
  • make sure edge cases like replying to an open thread which gets closed before you hit Submit are handled properly. We have checks to prevent you doing invalid things like starting a reply to a closed thread, but sometimes things can switch to an invalid state later, and we need to handle the errors they produce - if not a specific type of AwfulError, then make sure the popup tells you what's up at least. None of this generic "network error, please try again" stuff
@cloud8111
Copy link

Unhelpfull along with 10$ fee, amount successfully wasted.

@baka-kaba
Copy link
Contributor Author

What if the real awful error was inside us all along?

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

No branches or pull requests

3 participants