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

Audit SteamClientAdapter #35

Open
leonard-thieu opened this issue Dec 12, 2017 · 0 comments
Open

Audit SteamClientAdapter #35

leonard-thieu opened this issue Dec 12, 2017 · 0 comments

Comments

@leonard-thieu
Copy link
Owner

leonard-thieu commented Dec 12, 2017

SteamClientAdapter

Make it fully thread safe

SteamClientAdapter is currently only thread safe in certain scenarios. Making it thread safe would make it easier to consume and could allow it to support automatic reconnection.

It may also make sense to implement some sort of global (per local IP address) throttling alongside this so that attempts to connect don't immediately follow failures to connect. A failure to connect may indicate rate limiting by Steam. This currently isn't an issue given how SteamClientAdapter is currently used. SteamClientAdapter will wait to retry on a transient fault; however, this only guards against a single instance from retrying too soon. Different instances will be able to try to connect immediately.

It's important to note that each instance of SteamClientAdapter can only support 1 active connection at a time since the underlying SteamClient also has this limitation. An attempt to connect should not be made if already connected.

Cancellation support for LogOnAsync

LogOnAsync has been assumed to complete quickly but there is a lack of telemetry covering it. If this is not the case, adding cancellation support would allow it to return to the caller more quickly. This would in turn allow services to shut down faster when a Stop command is issued.

  • LogOnAsync should be instrumented to determine how long it takes to complete.
  • From that data, it can be determined whether it's worthwhile to add cancellation support.

Determine which callbacks are actually necessary

Verify which callbacks can actually run given possible connection states. It's important to determine the semantic meaning of a callback in a given state and what actions to take if it runs.

In particular, the onDisconnected callback in LogOnAsync has never been observed to run. This could be due to several reasons:

  • It has a very low chance of occurring (possibly due to LogOnAsync completing quickly)
  • It cannot occur. The failure state may be communicated through a different callback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant