Should I always add CancellationToken to my controller actions?
Asked Answered
M

5

75

Is this a good practice to always add CancellationToken in my actions no matter if operation is long or not?

I'm currently adding it to every action and I don't know if it's right or wrong.

[ApiController]
[Route("api/[controller]")]
public class DummiesController : ControllerBase
{
    private readonly AppDbContext _dbContext;

    public DummyController(AppDbContext dbContext)
    {
        _dbContext = dbContext;
    }

    [HttpGet("{id}")]
    public async Task<ActionResult<Dummy>> GetAsync(int id, CancellationToken ct) // <<----- should I always do this?
    {
        var dummy = await _dbContext.Dummies.AsNoTracking().SingleOrDefaultAsync(e=>e.Id == id, ct);
        if (dummy == null) return NotFound();
        return dummy;
    }
}

Also is adding CancellationToken ct = default(CancellationToken) necessary?

Moniz answered 14/5, 2018 at 11:51 Comment(13)
Add it if you are going to use it... If you don't plan on cancelling the operation, then why would you add it?Qualmish
@Qualmish I don't know. Maybe it would add some performance if user refreshes the page before server can fetch it from the database? Or it would be too fast to cancel?Moniz
Passing CancellationToken object doesn't magically cancel anything by itself. You have to cancel it yourself.Qualmish
@Qualmish in ASP.NET Core controller actions it does: andrewlock.net/…Moniz
"ASP.NET Core provides a mechanism for the web server (e.g. Kestrel) to signal when a request has been cancelled using a CancellationToken. This is exposed as HttpContext.RequestAborted, but you can also inject it automatically into your actions using model binding."Moniz
"MVC will automatically bind any CancellationToken parameters in an action method to the HttpContext.RequestAborted token, using the CancellationTokenModelBinder. This model binder is registered automatically when you call services.AddMvc() (or services.AddMvcCore()) in Startup.ConfigureServices()."Moniz
Ok, I didn't know that. It would be reasonable to add it to long running methods. If your method takes ~50ms, then it doesn't add any benefit.Qualmish
I don't get it. How is this opinion-based? The first two answers provide great insight with multiple references. The second best answer explains why it is a bad idea to always put cancellation tokens for actions and this is very important in some scenarios.Nahshun
@Alexei Then vote to re-open it pleaseSalicin
@Salicin Already did that.Nahshun
@Alexei I just see only one reopen vote, while I've voted as wellSalicin
@Salicin I think my initial one has expired. Voted again.Nahshun
@Salicin It got closed this time and you can post your answer. I have also posted a simplified version of this question on Codidact which is more tolerant of this type of questions (i.e. design choices which might also rely on one's opinion).Nahshun
B
58

Should I always add CancellationToken to my controller actions?

No. You should not always.

Using CancellationTokens in ASP.NET Core MVC controllers
https://andrewlock.net/using-cancellationtokens-in-asp-net-core-mvc-controllers/

Whether this is correct behaviour will depend on your app. If the request modifies state, then you may not want to halt execution mid-way through a method. On the other hand, if the request has no side-effects, then you probably want to stop the (presumably expensive) action as soon as you can.

So if you have a method/action like below (simplified);

await ProcessOrder();
await UpdateInventory();

You do not want to cancel the order while it is being processed, if so, order can be completed, but you will not update the inventory if user passing through the tunnel, and loses the internet connection.

This is especially important when operations cannot be included in a Unit of Work like pattern (e.g. distributed system) and one should try to minimize cancellations.

Bluebill answered 24/10, 2019 at 5:12 Comment(15)
Or you can use a transaction spanning both ProcessOrder and UpdateInventory. That way the request can still be cancelled if it's taking too long (and the client has retry / timeout policies configured for resilience), and the transaction will be rolled back leaving your database in a consistent state.Impedimenta
What if you are calling 3rd party api? Or doing I/O?Bluebill
Then you cannot guarantee consistency anyway, unless you have some cross-service transactional system going on (maybe event/callback-based with rollbacks on each service).Impedimenta
Then no need to use cancellations token for that. Just run to completion.Bluebill
Unless it's a resource intensive operation that you want to be cancelled when no one is waiting for a response. So let's just say it depends.Impedimenta
Resource intensive operations should be queued, and should be executed by separate processes.Bluebill
Maybe the calls to the API are not resource intensive themselves, but cancelling them might propagate the cancellation event to the second API.Impedimenta
"Might be". That can be a service there you don't have control of. Such as PayPal api, another payment/inventory system like Amazon etc.Bluebill
Like I said, it depends. But remember you can not guarantee consistency in this case.Impedimenta
This is not the point of neither the question nor the answer.Bluebill
I was adding nuance to your answer because you made it seem like consistency matters while your proposed solution is not guaranteeing itImpedimenta
You keep dragging the discussion out of the context. Consistency is a huge topic itself, and your suggestion against relational DBs with transactions is just a drop in an ocean. I did not mention it guarantees at all. I was referring the article with a dummy analogy.Bluebill
You are clearly talking about the inventory being consistent with the orders. So is the article you linked. It's not out of contextImpedimenta
Please post another answer if you think this one is not just not enough. I really would like to see and learn from your perspective. I am not here to discuss consistency algorithms and it’s implementations, therefore we will need to c/p entire white papers here. Thank you!Bluebill
Unfortunately, the question is closed (hopefully it gets re-opened soon), so I cannot expand the point in a comment very well. Here is the idea: imagine a user has called your api that runs a long-running db update via EFCore and you've passed CancellationToken to SaveChanges method of the context. If the token signals a cancellation (happens in application shutdown, HTTP connection abortion, tab closing, etc) then the running db transaction gets rolled back and an exception is thrown. Now, you should think what you must do in case an exception occurs from a DB operation.Salicin
V
24

It's worthwhile adding if you have any dependency on an external resource.

Let's say your database is busy, or you have transient error handling / retry policies in place for your database connection. If an end user presses stop in their browser the cancellation token will aid any waiting operations (in your code) to cancel gracefully.

Think of it less about the long running nature under normal circumstances, but long running nature in less than ideal circumstances e.g. if running in Azure and your SQL database is undergoing routine maintenance and entity framework is configured to retry itself (asuming Entity framework responds to cancellation tokens sensibly).

As a side note: Polly resilience framework has excellent support for external cancellation tokens to coordinate retry cancellations from an external cancellation. Their wiki is worthwhile a read as it's a great eye opener to coordinating cancellation: https://github.com/App-vNext/Polly/wiki

Regarding CancellationToken ct = default(CancellationToken), it's probably more worthwhile on library code than on a Controller action. But handy if you are unit testing your controller directly but don't want to pass in a cancellation token. Having said that .net core's WebHostBuilder testframework is easier to test against than testing controller actions directly these days.

Veradis answered 15/1, 2019 at 12:44 Comment(4)
Concerning unit testing the action method code that takes a CancellationToken, unless you are testing the cancellation itself some how just pass in CancellationToken.None.Mchenry
That's true unit testing shouldn't be the only reason default(CancellationToken) should be present.Veradis
Consider integration testing instead of unit testing your controllers. ASP.NET Core have great functionality for integration testing (which tests routing, model binding, authorization, etc).Highkey
Thanks Fred yes I totally agree, it's unlikely you'd want to unit test a controller these days, it's probably a slip of the tongue, I probably meant testing in general.Veradis
K
14

I think cancellation token should be used mostly for query type operation. Take CRUD (Create, Read, Update, Delete) operation as example, break it down based on CQRS (Command Query Responsibility Segregation).

  • Command: Create, Update, Delete
  • Query: Read

As you can see, for Query operation, it will not be a problem to cancel at any time. But for Command, you most probably don't want to cancel any of the operation. For example Create, imagine you are creating data for 2 separate databases, then got cancelled half way, it'll cause unsynchronized data.

Kaminsky answered 3/6, 2021 at 3:2 Comment(1)
Although I agree with this mostly, I think it's also worth to mention that Unit of Work concept can be considered in order to ensure data integrity.Bobbe
T
0

If you are not using it - neither checking it, nor passing it to something else - then it's just an unused variable.

But if you are making calls to the network, (that's your example):

They will often support a CancellationToken and you should take advantage - pass the one that ASP.NET Core can give you.

Let's suppose the database you are querying is very overloaded today. Calls will take forever -> YOUR user gets impatient -> keeps retrying -> all the old calls are still running -> overload gets worse and worse!

If you pass through the CancellationToken then, when your user's browser closes its connection, your related call to the slow remote system will end automatically.

Secondly, If you have a multi-stage operation or a compute loop,

You can check the CancellationToken manually, between stages or after every few runs of a compute loop. If user has stopped waiting, you can abort and stop wasting resources.

Tannenberg answered 22/12, 2023 at 3:11 Comment(0)
K
-1

Another approach is to cancel any operations, not just query operations.

When you copy a file on Windows and press the Cancel button, operation stops immediately. The same is for web applications. If user press a save button and then close the browser without waiting for operation to complete, then the Save operation could be stopped immediately.

For a long running operation UI could display a dialog with "Please wait" text and a cancellation button. That would make behavior more user friendly.

Kimsey answered 31/10, 2022 at 8:59 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.