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

WebAPI DefaultTimeout is not set to a reasonable default #993

Open
azuisleet opened this issue Jun 2, 2021 · 4 comments
Open

WebAPI DefaultTimeout is not set to a reasonable default #993

azuisleet opened this issue Jun 2, 2021 · 4 comments
Assignees

Comments

@azuisleet
Copy link
Member

The default timeout for WebAPI requests is set to a fixed 100 seconds currently, and there's no simple way to reconfigure it for classes like SteamDirectory that use the SteamConfigurationWebAPIExtensions to generate WebAPI interfaces from the SteamConfiguration.

https://github.com/SteamRE/SteamKit/blob/master/SteamKit2/SteamKit2/Steam/WebAPI/WebAPI.cs
https://github.com/SteamRE/SteamKit/blob/master/SteamKit2/SteamKit2/Steam/WebAPI/SteamConfigurationWebAPIExtensions.cs

There should probably be:

  1. A reasonable default closer to ConnectionTimeout (understanding that for HTTP requests, this is a full round trip timeout)
  2. The option to configure the timeout
@yaakov-h
Copy link
Member

yaakov-h commented Jun 3, 2021

  • Why do we need to reconfigure it?
  • What is a "reasonable" timeout?
  • Are you trying to lower or raise the timeout? ConnectionTimeout seems to be much lower.
  • Would something like making WebAPI.DefaultTimeout settable resolve the problem (if perhaps not ideal) or do you want different timeouts for different interfaces?

@azuisleet
Copy link
Member Author

azuisleet commented Jun 3, 2021

  1. If the user can configure all the other timeouts, including ConnectionTimeout and CDNClient timeouts, they should also be able to configure the timeouts that affect all WebAPI based requests.
  2. A reasonable timeout is probably closer to 30 seconds for a full request and response.
  3. I want to lower the timeout used to fetch the server list in SteamDirectory, as ConnectionTimeout is set to 5 seconds while the lookup is set to 100 seconds, as this affects how long CMClient.Connect takes (105 seconds in this case)
  4. Being able to set WebAPI.DefaultTimeout would be sufficient.

Also to be clear, if you own a WebAPI instance, there is a configurable Timeout property. https://github.com/SteamRE/SteamKit/blob/master/SteamKit2/SteamKit2/Steam/WebAPI/WebAPI.cs#L47

In this case, SteamDirectory owns the WebAPI instance https://github.com/SteamRE/SteamKit/blob/master/SteamKit2/SteamKit2/Steam/WebAPI/SteamDirectory.cs#L50

@yaakov-h
Copy link
Member

yaakov-h commented Oct 2, 2021

SteamDirectory owns it, and has a SteamConfiguration instance, so I'm tempted to add SteamConfiguration.WebApiTimeout so that it's not just some big static that people have to mutate.

Any thoughts on that?

@yaakov-h yaakov-h added this to the 2.5.0 milestone Dec 4, 2021
@xPaw
Copy link
Member

xPaw commented Mar 21, 2023

SteamConfiguration.WebApiTimeout sounds good because you can get instances of WebAPI from Configuration.

@xPaw xPaw modified the milestones: 2.5.0, Some day (PRs welcome) Mar 21, 2023
@yaakov-h yaakov-h self-assigned this Mar 21, 2023
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