Async best practices - CancellationToken, .Result

[jonstelly]

.Result

All of the references to task.Result should probably be changed to task.ConfigureAwait(false).GetAwaiter().GetResult() to help callers avoid deadlocks

Cancellation

(All?) Async methods should take a CancellationToken parameter to support cancellation. In most cases the parameter can probably be added with a default like this:

public async Task<RegisterDomainResponse> RegisterDomainAsync(RegisterDomainRequest body)

becomes

public async Task<RegisterDomainResponse> RegisterDomainAsync(RegisterDomainRequest body, CancellationToken cancellation = default)

and cancellation gets passed through to any methods that take a CancellationToken parameter.

[shaofu88]

Thanks for the feedback. We have an action item to look into this and see what we can do in the upcoming releases.

[jonstelly]

Thanks. One other suggestion... all of the example code where .Result is used, just change the examples to be async. I bet most people using this SDK will be using it from an async context, and if someone needs the non-async they can convert to using the GetAwaiter().GetResult(). This keeps your examples showing the suggested way to use the library and if someone is copy/pasting code then they'll at least get pushed in the right direction:

var body = new RegisterDomainRequest.Builder(
        "example.com")
    .Build();

try
{
    RegisterDomainResponse result = applePayApi.RegisterDomainAsync(body).Result;
}
catch (ApiException e){};

becomes

var body = new RegisterDomainRequest.Builder(
        "example.com")
    .Build();

try
{
    RegisterDomainResponse result = await applePayApi.RegisterDomainAsync(body);
}
catch (ApiException e){};

[jessdelacruzsantos]

The cancellation token parameter has ben added.