lutris / lutris

Lutris desktop client
https://lutris.net
GNU General Public License v3.0
7.84k stars 690 forks source link

Make the services connection errors consistent #5406

Open strycore opened 6 months ago

strycore commented 6 months ago

The way our integrations deal with being logged out / expired token / invalid credentials is a complete mess. No 2 services behave the same and I seem to remember only one or 2 happen to do the right thing.

Here's a bunch of stuff we're doing wrong

Anyway, this area of lutris has been chaotic, ugly, nondeterministic and offensive. We all share the blame for letting that happen.

danieljohnson2 commented 5 months ago

I see room for improvement here, so I've made the 'Refresh' button log you back in if you are not found to be connected, which will ask you for a password, then reload anyway. Saves you the click you were almost certainly going to make.

I've also added an interlock so if the login process is abandoned, even with cookies remaining, we don't consider that a successful login anymore. You've got to reach login_callback() or it won't count.

But I feel like there's more wanted here, and it's not clear to me. Do we want to preemptively check each source at some point to detect if you have been disconnected? I don't think we want to just poll all the sources all the time.

strycore commented 5 months ago

No polling needed, I would just like services to stop showing "token expired" errors and just show the login window instead.

danieljohnson2 commented 5 months ago

Hmm. I don't have the ability to really test this very well, but I think we can get closer here.

I've changed Lutris to just use one AuthenticationError class, and to have a backstop that will log out and log in again if such an error hits the backstop. That should help.

There are services that never raise this error. Origin for instance. So maybe we aren't done here.

strycore commented 5 months ago

I don't have the ability to really test

Well that should make you life easier. If you can't test then you can't change.

Origin for instance

Origin can be removed.

danieljohnson2 commented 5 months ago

I tested this by sabotaging it- deleting credential files while Lutris was running. It's not exactly the same as being expired by the service, but I managed to run the offending code path.

I can just revert it all if you feel that's not good enough. I dunno how you're going to test this, but I'm sure you'll figure something out.

strycore commented 5 months ago

I can test because I have accounts on all services we propose but yes, it is annoying to reproduce.

danieljohnson2 commented 5 months ago

I'll take that as "revert it all and leave it to you."