Add async/await support for job based messages

[voided]

So here's an experiment that might be worth mainlining.

The idea here is that we introduce an AsyncJob object that inherits from JobID, and have all our job related client message handler functions return this object. Client code will continue to compile since existing code can work with the JobID superclass just fine.

The AsyncJob objects expose a GetAwaiter() to support awaiting directly on that object, and a ToTask() to get the underlying TPL task that the job itself is wrapping.

Awaiting the result of the job will directly return the callback object for that job, i.e.: awaiting a call to GetAppOwnershipTicket will return a AppOwnershipTicketCallback without any callback subscription boilerplate.

The AsyncJob instances are created by client message handlers, and get tied to the handlers' parent SteamClient instance. SteamClient then handles the job's completion through PostCallback: when a callback for that JobID is posted, SteamClient completes the job and await returns the callback.

SteamClient is also tasked with timing out any jobs that never receive a response from Steam. The default timeout is 1 minute, but can be managed with the AsyncJob.Timeout property. Any jobs that don't receive a response are canceled and can be handled through regular exception handling.

There's also an additional AsyncJobMultiple for the instances where Steam can reply with multiple messages to a single job based request. In this case, awaiting on this job will instead return a ResultSet object that contains a list of the retrieved callbacks. There's also a few cases involved with this one:

1. If Steam replies with all the messages expected for the request, the ResultSet is returned with all the messages and Complete will indicate the result set is complete.
2. If Steam replies with only a few messages (and then the timeout is reached while waiting for the rest of the set), the ResultSet will be returned with what the client managed to receive and Complete will be set to false.
3. If Steam doesn't reply to the request at all, the job is entirely canceled and the exception should be handled.

Caveats:

• Bumping up our .NET requirement to 4.5 or Mono 3.0 (which was released Oct 19, 2012).
• Some of the new unit tests require async/await support, which is only supported with xunit 2.0.0.
• xunit 2.0.0 and Travis CI's current stable build of mono have issues (see xunit 2.0 TypeLoadException on Mono xunit/xunit#158), so we inform Travis that we need an alpha version of mono (4.2) to run our tests.

[Netshroud]

we introduce an AsyncJob object that inherits from JobID, and have all our job related client message handler functions return this object. Client code will continue to compile since existing code can work with the JobID superclass just fine.

I don't like this bit... it doesn't make sense to me for a job to be a subclass of the job id from a pure design perspective, it seems like the only justification for this is backwards compatibility.

Compilation compatibility could also be achieved through an implicit cast operator, but that would fall over for consumers that use var instead of declaring their receiving type, if they use JobID directly and don't just immediately pass it into a function that takes JobID as a parameter. For the 99% use case, this ought to suffice.

Otherwise, I'd rather make a breaking change and bump the major version (finally?).

[voided]

We still need to keep JobIDs around for matching callbacks to requests into the far foreseeable future, even for a major version bump I wouldn't want to make a breaking change like that. Some sort of implicit cast sounds compelling, though.

[voided]

After thinking about it a little, I don't think any implicit casts will work here either. Consider:

AppOwnershipTicketCallback callback = await steamApps.GetAppOwnershipTicket( 440 );

I don't think the compiler will be able to work out that GetAppOwnershipTicket's return needs to be casted into an AsyncJob.

[Netshroud]

I was thinking the other way around.

AsyncJob asyncJob = steamApps.GetAppOwnershipTicket( 440 );
AppOwnershipTicket callback = await asyncJob;

void Foo( JobID jobID ) { ... }

JobID myJobID = steamApps.GetAppOwnershipTicket( 440 );
Foo( myJobID );

void Foo( JobID jobID ) { ... }

var /* actually an AsyncJob now */ myJobID = steamApps.GetAppOwnershipTicket( 440 );
Foo( myJobID );

[voided]

Okay, with this we now have some initial handling for heartbeats and remote failures (from #171). I've also switched it over to an implicit cast, which I think retains the same amount of source code compatibility as before.

All that's really left is to implement the logic and tests for remote failure exceptions.

[xPaw]

This branch has been running in production at @SteamDatabase for a couple of days now, and makes use of await, no issues.

[Netshroud]

"The name of an async method, by convention, ends with an "Async" suffix." - Asynchronous Programming with Async and Await (C# and Visual Basic)

Worth considering, at the risk of breaking backwards compatibility?

[voided]

Far too much breakage in my opinion.

Edit: Here's a choice quote from Stephen Toub on the convention:

Of course, there are always exceptions to a guideline. The most notable one in the case of naming would be cases where an entire type’s raison d’etre is to provide async-focused functionality, in which case having Async on every method would be overkill, e.g. the methods on Task itself that produce other Tasks.

In our case, our entire design is already around asynchronous method completion, so I'd agree with the overkill sentiment.

[xPaw]

Not directly related to this PR, but perhaps look into making CDNClient async too?

[keichinger]

What is the reason for the AsyncJob<T> class? Why is it used over Task<T>? When working with async/await one should also not forget about implementing CancellationToken which is the preferred way to cancel async operations. While I like the idea of having async operations available I strongly encourage to stick to existing classes unless there is a good reason not to do do.

If it's due to backwards compatibility then I'd either recommend splitting this feature into a dedicated NuGet package or following SemVer's guidelines and release a new major version that explicitly allows breaking backwards compatibility changes.

[voided]

Backwards compatibility is indeed the reasoning. We don't have any plans to make any significant breaking changes (such was removing our entire callback scheme for job based messages) any time in the future. Having a separate library version that follows the TPL conventions more closely is an interesting idea, but I wouldn't be happy with the maintenance burden.

[codecov-io]

Current coverage is 0.00%

Merging #170 into master will decrease coverage by -17.87% as of 180ca80

No diff could be generated. No reports for #170 found.
Review entire Coverage Diff as of 180ca80

Powered by Codecov. Updated on successful CI builds.

[xPaw]

It might be a good idea to turn this into a callback too, so that users not using async can see catch their failed jobs.

[voided]

There's #171 regarding this, but I'm not sure if consumers really want to go through the code gymnastics of handling failures (and likely heartbeats) by having multiple callbacks.