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

WIP: Show non-modal infobar for some errors #612

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

TingPing
Copy link
Member

@TingPing TingPing commented Apr 1, 2020

TODO:

  • Since this is no longer modal we need to hide the infobar when the error is bypassed. I guess we effectively need to track a "connected" state.
  • We may want to retry automatically in some situations
  • We can show better buttons, like on auth failure show preferences not retry
  • Some pandora errors deserve the dialog?
  • Ellipsize long messages, and add tooltip for full message?

The pandora error messages frankly suck, maybe we should add our own, bit out of scope for this PR...

See also #611

TODO:
- [ ] Since this is no longer modal we may need to hide the infobar when the error is bypassed
- [ ] We may want to retry automatically in some situations
- [ ] We can show better buttons, like on auth failure show preferences not retry

See also #611
@TingPing TingPing changed the title WIP: Show non-fatal infobar for some errors WIP: Show non-modal infobar for some errors Apr 1, 2020
@JasonLG1979
Copy link
Collaborator

The pandora error messages frankly suck, maybe we should add our own, bit out of scope for this PR...

In the event of a connection issue those errors more than likely aren't pandora errors, after all it means we can't connect to pandora. Those are urllib.errors. We could maybe translate them into less technical terms?

@JasonLG1979
Copy link
Collaborator

  • Since this is no longer modal we need to hide the infobar when the error is bypassed. I guess we effectively need to track a "connected" state.

We already track if we're connected. pandora.connected. In the event of some pandora api errors and pretty much all urllib.errors we can basically assume that we're no longer connected.

  • We may want to retry automatically in some situations

We can do that pretty easily with pandora_reconnect

  • We can show better buttons, like on auth failure show preferences not retry

Again pretty easy. Pretty much all of the known pandora api error codes are already enumerated.

  • Some pandora errors deserve the dialog?

In the event of network errors we should show the error in the infobar and retry a couple times with long enough times between retries that the network has a little time to recover. Maybe like 3 tries a min apart then show a dialog?

Pandora api errors are a whole different animal then network errors. There are a few that should not result in a retry and some that no amount of reties will help. Again though most of the common api errors already have sub messages in the ApiError class.

@JasonLG1979
Copy link
Collaborator

JasonLG1979 commented Apr 4, 2020

Off the top of my head:

These errors should not be retried and should pretty much be considered fatal at least temporarily:
MAINTENANCE_MODE
INSUFFICIENT_CONNECTIVITY
READ_ONLY_MODE
PARTNER_NOT_AUTHORIZED
DEVICE_MODEL_INVALID

These errors should present a "hey dummy" dialog as these are the direct result of user interaction:
MAX_STATIONS_REACHED *non fatal you just can't add anymore stations.
PLAYLIST_EXCEEDED *fatal temporarily at least for that account.

These errors should present the appropriate user setting and retry after user interaction:
COUNTRY_NOT_SUPPORTED *proxy settings
INVALID_LOGIN *prefs dialog.

@TingPing
Copy link
Member Author

TingPing commented Apr 5, 2020

If you want to see this work through that would be great, I'm not sure I'll find the time.

@JasonLG1979
Copy link
Collaborator

If you want to see this work through that would be great, I'm not sure I'll find the time.

Sure, I'll see what I can come up with next weekend. With the THING going on I'm trying to get my kids setup for at home learning this weekend. They haven't said so yet but I think school is done for the year. I'm also finalizing a thorough (read: tweaker) cleaning routine for them since they basically can't leave the house and I'll be damned if I have to ride out the apocalypse in a dirty house,lol!!!

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

Successfully merging this pull request may close these issues.

None yet

2 participants