webUISessionTimeoutInput user setting, a good idea? #10892
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.
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.
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.
I don't think I have further plans for it.
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.
To be honest, I don't understand exactly what the problem is.
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.
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).
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.
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.
Your client sends a bunch of requests without checking their result?
Client can query for appropriate settings once it is connected to qBittorrent. Isn't it?
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.
LOL some one actually gave it a name: Hyrum's Law.
Hi, I've been out a few hours, sorry for the delay...
Yeah, my example was an over simplification to try and remove confusion. Sorry, I seem to have created more - doh!
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.
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.
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.
I'm sure people would rather use your latest efforts :-)
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.
@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).
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...
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.
To avoid further misunderstanding, I will reiterate the basic idea.
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.
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
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?
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.
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.
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.
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.
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.
You can read over my thoughts about the
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.
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 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.
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.
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 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".
You cannot hold yourself from making accusations at any opportunity, again you have done it...
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.
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.
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.
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.