webUISessionTimeoutInput user setting, a good idea?

[JackDandy]

Please provide the following information

qBittorrent version and Operating System

An upcoming version (the change is on master branch)

What is the problem

Reference: Add WebAPI session timeout settings commit#89124bd

webUISessionTimeoutInput is/was a static 60 mins. A client could re-authenticate every 45 mins and all requests in between would operate using the authenticated session, great.

However, the change to make the timeout value user defined leads to every client request requiring an additional pre-authentication request.

Why?

A third party client connecting to qB API can be running under the assumption of a qB timeout at 60 mins (w/45 mins session reauth interval).

But if a user changes the qB setting from 60 to 10, all requests will now fail as client is not due to re-authenticate until up to 45 mins have elapsed, user could easily think that there is an issue with client.

Solutions...

• Client could add a qB timeout to its own UI, this is not great, can qB expect a) all clients to add this? and b) users to know to set it?
• Client could read the qB->app/preferences to find the value, but auth is needed for that call too, so failure is inevitable. And even if it wasn't, it would simply need to be called at each request anyway.

Therefore, the simplest route for client to take is to authenticate before every API request to preempt any race issues where the qBs->webUISessionTimeoutInput value is decoupled from an internal client timeout property value.

Of course, this is not ideal given that the client was using minimal resources by only re-authenticating every 45 mins, and now has to reauth at every request, each request now becomes two as it cannot be determined when in time a user changes the qB setting value.

What is the expected behavior

Revert the value to be internally hardcoded so that third party clients that depend on it won't break.

I assume this change is here to stay, and have taken my simplest route, unless you have a nicer recommendation.

Thanks.

[glassez]

@Chocobo1?

[Chocobo1]

@Chocobo1?

I don't think I have further plans for it. Some users requested the current behavior and I think it is reasonable too, and still someone thinks otherwise... There exists many non-ideal solutions such as 3rd party programs telling their users not to fiddle with the session timeout settings or teach them to set to 0 (no timeout), but I'm afraid 3rd parties will still think it is too cumbersome.

[JackDandy]

and still someone thinks otherwise

It's not an opinion. I demonstrated how the change affects clients/users in a practical way.

Sometimes, changes have unintended side-effects. I thought that may be the case here, and simply thought to bring it to your attention rather than say nothing.

I considered waiting until a 403 is returned from the API to trigger a call to reauthenticate, but that feels like a brute force approach, and is not so ideal to spam 403's in logs, therefore, being proactive for max. stability, SickGear will authenticate before every request.

[glassez]

To be honest, I don't understand exactly what the problem is. If the client has established a session and it is using it continuously (that is, performing some requests at least as often as the configured "session timeout" limit), the session will never expire until the client stops using it. And there is no re-authentication required. If the client does not retain the session, the session expires after some time, what is wrong with that?

[JackDandy]

Yes, you are correct in the use case you are thinking about.

To the case I raised.. existing third party clients are aware that the timeout is 60 mins and this known value means they don't need to reauth a session until, for example, 55 mins used.

However, now that the timeout value can be lowered, the client will still be using a 60 min threshold, and will send a bunch of API commands beyond any user lowered timeout value, each of those requests will now fail. Plus, as there is no way to determine a users change to the timeout value, two calls per request is now needed to achieve a reliable API result.

Hope I have made it clearer.

[glassez]

Hope I have made it clearer.

Really, you did it. Now I clearly see a client with a bunch of shortcomings in its implementation (if you, of course, mean some real client).

clients are aware that the timeout is 60 mins and this known value means they don't need to reauth

AFAIK, this value was never published. It was just one of application internals which can be changed at any version. So it's not correct to blame us for changing it.

the client will still be using a 60 min threshold, and will send a bunch of API commands beyond any user lowered timeout value, each of those requests will now fail.

Weird behavior, IMO. You create the session, don't use it for 60 min and then "send a bunch of API commands". At least it seems correct for me closing unused session. Just reestablish it when it's needed again.

each of those requests will now fail

Your client sends a bunch of requests without checking their result?

Plus, as there is no way to determine a users change to the timeout value

Client can query for appropriate settings once it is connected to qBittorrent. Isn't it? Or are you worried that this setting will change during the lifetime of the session? Just don't change it. Although qBittorrent supports many concurrent sessions, it is not really multi-user. It is supposed that all sessions belong to the same user.

existing third party clients

If you really worried about some really "existing third party clients" there's nothing to be done... We cannot add some improvements/fixes without breaking compatibility. But even in this case you have options - either use one of the qBittorrent versions that is supported by your client, or use the new version with compatible settings.

[Chocobo1]

clients are aware that the timeout is 60 mins and this known value means they don't need to reauth

AFAIK, this value was never published. It was just one of application internals which can be changed at any version. So it's not correct to blame us for changing it.

LOL some one actually gave it a name: Hyrum's Law.

[JackDandy]

Hi, I've been out a few hours, sorry for the delay...

You create the session, don't use it for 60 min and then "send a bunch of API commands". At least it seems correct for me closing unused session. Just reestablish it when it's needed again.

Yeah, my example was an over simplification to try and remove confusion. Sorry, I seem to have created more - doh!

Weird behavior, IMO. You create the session, don't use it for 60 min and then "send a bunch of API commands". At least it seems correct for me closing unused session.

Yeah, that's not really what I meant. Okay, sure a client should see an API response is an error, and act on that, but the session is running on, okay, let's change this and say, a ~30 min timeout, and therefore, another API request is made using the existing session. When that session expires, a new 30m auth session is made, but requests are good up until, for example, a user defined 10 mins, and then client will see errors from 10 mins until 30 mins. Yes those errors are caught, but they would still occur. And the client didn't read your mind to know that these errors could ever even mean to "reauth", therefore, a logic improvement PR for client will be needed. I guess time will tell to see what will probably only be a small impact this change has, and maybe other devs see this thread and make required change so that the impact is minimal overall - that would be a good outcome from this thread.

Just reestablish it when it's needed again.

Many things reuse a connection, HTTP, DBs etc. they don't reestablish a connection at each point of communication due to a period of inactivity. However, yes, yours is exactly what my OP suggestion for qB is.

Client can query for appropriate settings once it is connected to qBittorrent. Isn't it?

Yes, and this too will require a change to existing client logic. But even then, if client reads the timeout config value with a required authenticated session, finds it at 30 mins, and then user reduces it to 10 mins, while the client is on a 30mins timeout, then really the client should read the setting at every request, and well, if the client is doing that, then it may as well simply never reuse a session for any request, and instead, the client should change their logic to reauth per any request made, which is why I made that exact suggested solution in my OP.

either use one of the qBittorrent versions that is supported by your client

I'm sure people would rather use your latest efforts :-)

...or use the new version with compatible settings

As you said, it's not public what those "compatible settings" are, in this case that would be 60mins, and i don't know if users will even remember to leave theirs at 60mins, or set it to 0 - or if it will even matter. Having this option available does open a new potential for this issue.

Hopefully, the possible side effect from this change is highlighted. That is, if a client is reusing a session for max. efficiency with the qB API expecting the session to stay valid for even 20 mins since the last request, this will no longer be a valid assumption as a user will be able to set a session to last say 20 mins, and then make another reduction as low as 1 min (currently), so the client supporting the qB API should make provision for this.

[glassez]

@JackDandy, I'm not sure I understood everything you wrote above correctly. I'm not a native English speaker, so it's hard for me to understand poorly formalized explanations.

I am very confused by a "mystic" user who prevents your client to work correctly. I don't understand why you separate them so much. It looks to me like you're trying to break it on purpose. Because in normal use, there is only one user. It installs/configures the qBittorrent and it also uses it in some way (e.g. using some web client). The problem with changing the discussed option using your existing client is also not so clear for me. If it doesn't know about it how it can change it?

[JackDandy]

This is nothing about a "mystic user", and is not me trying to intentionally break things, or anything childish like that, I do not waste my time with those things. I'm trying to help you, and this is what you say to me, plus other tones that I have ignored, sigh.

Okay, maybe the following snippet might help...

def _qb_request(self)

    if time.time() > self.last_time + 1800:
        self.last_time = time.time()
        self.get_auth()
    ...
    ...
    self.generic_request()
    ...
    ...

The function reduces the number of API calls by authenticating only once every 30 minutes.

This pattern will break because the API also respects the timeout.

Simple.

[glassez]

I'm trying to help you, and this is what you say to me, plus other tones that I have ignored, sigh.

To avoid further misunderstanding, I will reiterate the basic idea. The changes we discuss here are made to implement/improve some other features requested by users so that they will not be reverted. If such changes break the compatibility of some 3rd-party clients, then this is a perfectly acceptable phenomenon, since it can not be avoided (especially if they break the compatibility of clients implemented on the basis of some false assumptions and using some non-public things). What options exist in this situation until these clients are updated, I described above. You can't really help me (not me really, but qBittorrent project) suggesting to revert this changes. All that can be done in this case is to try to find some opportunities to improve this feature a bit without breaking its functionality.

As for the rest of our discussion, where I'm trying to understand your client's logic, it's more for fun. Although it may give some practical results.

Okay, maybe the following snippet might help...

This, of course, is not enough to see the whole picture of what is happening. For example, it would be nice to know under what conditions and at what intervals you call _qb_request() (i.e., the logic of the upper level). But even the code you gave me confuses me. Why do you rely on some time interval? Why don't you just send the request and re-authenticating if it fails due to appropriate error? E.g (pseudocode):

status = generic_request()
if (status == unauthenticated)
{
    auth_request()
    generic_request()
}

[JackDandy]

(especially if they break the compatibility of clients implemented on the basis of some false assumptions and using some non-public things).

Hey, the qB codebase is open source and public for all to read. There is an expression, if it looks like a duck, walks like a duck, and quacks like a duck, then it is a duck. Your "false assumption" remark is naive... an assumption cannot be made when the API actually behaves in the way it has for years, the code does a timeout at 3600 seconds, this is clear to read in public - therefore, it is what it is - that is, factual and not assumption. But the API session timeout is now about to change to be something different - so developers that support your API should be made aware, that's just being nice and helpful to us - no?

If such changes break the compatibility of some 3rd-party clients, then this is a perfectly acceptable phenomenon

Yes that may be acceptable for you, but don't you feel it considerate to inform people of breaking API changes when someone highlights an issue to you, so that we can change and not have users wonder why your app suddenly breaks with our apps? Maybe you don't, that's your prerogative.

All that can be done in this case is to try to find some opportunities to improve this feature a bit without breaking its functionality.

Good luck with that. The Application Programmers Interface API is what developers use, therefore, developers will naturally test the interface in order to work with its character. I am helping to make aware that your change may break an implementation, and how, and what could be done before hitting a wall - sure, there are other solutions too.

You can't really help me (not me really, but qBittorrent project) suggesting to revert this changes.

The suggestion to revert the change was simply one of. More importantly, you will see that I followed that by writing "I assume this change is here to stay", and I submitted one suggestion that a developer could take, as a way to raise ideas, not as something definitive, and I clearly was not advocating a revert when I wrote that, I was offering a positive way forward.

Of course I thought about possibilities other than a revert, but I could not see any way that will not break the pattern I highlighted.

For example, it would be nice to know under what conditions and at what intervals you call _qb_request() (i.e., the logic of the upper level).

It doesn't matter how often or when the pattern is executed, it will break. Therefore, the API developer may need to make change on behalf of their users.

Why do you rely on some time interval?

I suppose an answer to this is - why not? I could ask, why was there a 60 minute timeout interval in qB to begin with? But does it really matter!? I think we are where we are.

Why don't you just send the request and re-authenticating if it fails due to appropriate error?

You can read over my thoughts about the HTTP 403 solution (that is; status == unauthenticated) in my 2nd post, in short, I'm not convinced it is wise to spam a 403 authentication error into the logs whenever a generic_request() is made after a session timeout. This is why I prefer to reauthenticate on each request and keep logs clean so that if a user sees a 403, they know that there is a genuine authentication abuse and not a false alarm.

Bottom line is the same as in my OP, the feature change is here to stay, I have never doubted that, and anyone currently using a similar pattern in code that I showed above needs to change.

[glassez]

Looks like you're overreacting. Your client supports a specific range of qBittorrent versions. Well, this one's intact. Moreover, your client can still support new versions, because although this property has become customizable, the default value remains the same (AFAIK), and if you do not touch it, nothing bad will happen. Sorry, I have nothing more to add. Maybe @Chocobo1 has to say something other...

[JackDandy]

Looks like you're overreacting.

No, I'm not overreacting, I simply answered your snark dismissive tones - to the point where you accuse me of overreacting, it's not dignified of you.

and if you do not touch it, nothing bad will happen.

And as was already said by chocobo1, "There exists many non-ideal solutions such as 3rd party programs telling their users not to fiddle with the session timeout settings"

The feature is added to qB, we must assume users will use it, therefore, existing patterns may break. Us developers cannot ignore the change, or tell users not to use it, like how we cannot expect you to revert it. Therefore, developers will need to make change, as I reported in my original post.

Sorry, I have nothing more to add.

You added nothing, except to consistently play down my findings and help. All you needed to do was acknowledge to inform third party developers who support this project, but it seems you won't. Pity.

[glassez]

All you needed to do was acknowledge to inform third party developers who support this project, but it seems you won't.

Before the publication of your "angry" post, I only knew about one 3rd party client, the author of which clearly declared himself and at least a little participated in the development process. I do not see any other way to inform unknown 3rd party developers, except for the publication of information on the official website. You can see this in the News section when a new version (containing these changes) is released.

I simply answered your snark dismissive tones

I was just trying (without any emotions j to present you the starting point from which to start the future of this feature (since you missed the opportunity to participate in the discussion at the stage of PR). Too bad you thought it was "dismissive tones". I encourage you to participate in the development process, but do not scold the developers for the work done and "regret the past".

[JackDandy]

I said...

All you needed to do was acknowledge to inform third party developers who support this project, but it seems you won't.

You replied...

Before the publication of your "angry" post

You cannot hold yourself from making accusations at any opportunity, again you have done it...
No it was not an angry post, I described how you communicate, how you have been snarky, and when I called you out, you called that "overrecting", and when I called you out on that, you call it "angry" - so no, I am not angry, I simply understand when people try to be "clever" in text.

I only knew about one 3rd party client

You have an API. Many clients will use the qB API. qB is not my app, and I know many clients that use it. You clearly expect more than one client is using it, to say "one" is nonsense.

I do not see any other way to inform unknown 3rd party developers, except for the publication of information on the official website

Yes, I guess it is how you inform of the feature, if you only write, "new: user configurable timeout" then people will not appreciate the implication of the change. However, if you add something like, "an API session can now timeout never(0) or from 1 min and more, a change from the hardwired 60 mins", then this would be a very transparent way that could hugely help manage this potentially breaking change for the qB API developers, and your userbase that use their apps to connect to qB.

I encourage you to participate in the development process,

Thank you, but I cannot keep up with every project day-to-day progression, we have our own matters to attend to. We can simply try to give help by informing about findings when they are observed.

but do not scold the developers for the work done and "regret the past".

To be clear, I have not scolded you, I have talked in a manner of facts. Someone used to say, "the truth hurts", I think you need to be less precious when people raise issues to you, And no, I am not "regretting the past", I am only able to tell you my findings as I see them now while I am updating code to support qB API changes for the SickGear<->qB userbase.

[glassez]

If you only used a public API and didn't rely on the app internals (as a correct client should), you wouldn't be in this situation. We have not made any breaking changes to the API, so no client using only the public API should be affected.