2
\$\begingroup\$

In Shopify a product can belong to a collection.

I have written a method which accepts an organisation. It would then get a list of collections that this organisation has from Shopify. Finally, it has to loop through each collection to get the products that belong to this collection.

I don't want to overload Shopify server by sending too many requests, so I have added 75 ms wait between each request. But since this method is async, I am not quite sure if the responses are handled correctly? Do I need any sort of mutext or locking mechanism to ensure each response is being handled correctly?

public async Task<List<ShopifyCollection>> GetShopifyCollections(Organisation organisation)
{
    int waitInMilliSeconds = 50;
    var shopifyCollections = new List<ShopifyCollection>();
    var customCollections = await GetAllCustomCollections(organisation);

    foreach (var cc in customCollections)
    {
        shopifyCollections.AddCollection(cc.Id, cc.Title);
    }

    foreach (var curCollection in shopifyCollections)
    {
        var products = await GetCollectionProducts(organisation, curCollection.CollectionId);
        curCollection.ProductIds = products.Where(p => p.Id != null && p.Id > 0).Select(p => p.Id.Value).ToList();
        await Task.Delay(waitInMilliSeconds);   // <-- do not overload Shopify 
    }

    return shopifyCollections;
}
\$\endgroup\$
4
  • 2
    \$\begingroup\$ In async code you should use Task.Delay docs.microsoft.com/en-us/dotnet/api/… \$\endgroup\$ Commented Sep 6, 2020 at 1:27
  • \$\begingroup\$ @CharlesNRice: thanks for your comment, I have modified my code to use Task.Delay instead of Thread.Sleep \$\endgroup\$ Commented Sep 6, 2020 at 3:56
  • 1
    \$\begingroup\$ Do I need any sort of mutext or locking mechanism...? No. I am not quite sure if the responses are handled correctly Why do you think that the responses aren't correctly handled? As for me, this method looks fine now. \$\endgroup\$
    – aepot
    Commented Sep 6, 2020 at 20:58
  • \$\begingroup\$ @aepot: thanks for your response, I have never used Delay in an async method and just wanted to double check if this is correct. \$\endgroup\$ Commented Sep 7, 2020 at 3:11

2 Answers 2

4
\$\begingroup\$

Create throttling for API calls is just one aspect of making your API integration resilient. There are many other types of failures to handle. Instead of weaving the error handling into your code, you should consider using something like Polly https://github.com/App-vNext/Polly

There a more details on it here http://www.thepollyproject.org/2018/03/06/policy-recommendations-for-azure-cognitive-services/

\$\endgroup\$
2
\$\begingroup\$

But since this method is async, I am not quite sure if the responses are handled correctly?

Your observation on asyncs not inherently waiting is correct, but you've glossed over the await which enforces the waiting logic.

Were you to have written this code:

Task.Delay(waitInMilliSeconds);

The could would indeed not wait. The task would be returned immediately, and your code here would continue immediately.

But that all changes when you use await:

await Task.Delay(waitInMilliSeconds);

Now, the task will be returned immediately, the await will block your current code until the task is completed, and your code here would continue after the await has stopped blocking it.

Note that during the wait, control is released outside of your code, so it is possible that other parts of your application keep working in the meantime if they are not themselves awaiting the completion of another task.

\$\endgroup\$
2
  • \$\begingroup\$ Great explanation, thanks a lot. \$\endgroup\$ Commented Sep 7, 2020 at 11:26
  • \$\begingroup\$ will block your current code until = will schedule the continuation of the code after. Actually it doesn't block anything. Technically it leaves (ends) the method and returns to execution after awaiting is done. Sorry, I cringe when someone locks or blocks something in async code. ) \$\endgroup\$
    – aepot
    Commented Sep 9, 2020 at 19:03

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.