Add CancellationToken support to QueryAsync overload

[itcotta]

This change introduces an optional CancellationToken parameter to the QueryAsync method, enabling support for cancellation of asynchronous database queries. The modification ensures better resilience and control over long-running or potentially cancelable operations.

This fixes issues #2125 and #1938.

[PauloHMattos]

This would be a breaking change. I don't think this can me added until a new major release of Dapper

[itcotta]

This would be a breaking change. I don't think this can me added until a new major release of Dapper

Hi, I'm a bit unsure why this would be considered a breaking change as the new CancellationToken parameter is optional when calling the QueryAsync method. That said I'm new to this code base, perhaps I'm missing something? However, if this is really a breaking change, then I agree that it would make most sense to wait with merging this until the next major release.

[worming004]

This is in detail, and most C# developers accept this change as it is not breaking in a common way. But there is edge cases adding an optionnal parameter is a breaking change. For example:

Here is startup case

public class Foo
{
    public static async Task Bar()
    {
        await Task.Delay(5);
        System.Console.WriteLine("Bar");
    }

}

public class MyProgram()
{
    public static async Task MyMain()
    {
        Func<Task> MyFunc = Foo.Bar;

        await MyFunc();
    }
}

When modifying to public static async Task Bar(Cancellation Token = default), then MyProgram cannot compile anymore. MyFunc expect Foo.Bar to have a definition matching Func<Task>. But the only definition is now Func<CancellationToken, Task>

Here is the change without breaking change:

public class Foo
{
    public static Task Bar()
    {
        return Bar(default);
    }

    public static async Task Bar(CancellationToken cts)
    {
        await Task.Delay(5, cts);
        System.Console.WriteLine("Bar");
    }
}

public class MyProgram()
{
    public static async Task MyMain()
    {
        Func<Task> MyFunc = Foo.Bar;

        await MyFunc();
    }
}

This is not breaking anymore.

[mgravell]

To be clear: any change to the parameters is always a breaking change.

Fundamentally, there are two very different kinds of "break" we need to consider:

• source breaking changes - things that prevent new compile runs from completing
• runtime / "binary" breaking changes - things that prevent existing code compiled against an old version of the library from running against a newer version of the library

Changes can be either or both (or neither) kind of break. We need to be mindful of both kinds of breaks, but since we're a commonly used library we need to be especially mindful of the second kind: the first kind of break causes a developer to have a bad 20 minutes while they change 3 lines of code with whatever change needs applying, but the second kind causes a department or entire company to have a bad day or days when a service randomly goes offline when they updated an innocent looking unrelated package that caused a transitive library update 3 steps removed, leading to MissingMethodException or similar.

So: we don't want that.

Adding a parameter - even an optional parameter - is a binary breaking: existing code compiled against the old method will simply stop working if the library is updated in this way, unless all code using it is recompiled. The compiled IL indicates a very specific method - one that will no longer be found (optional parameters don't make any difference for IL bind purposes). It is not always (or even often) the case that every package is rebuilt for every deployment - and often, there may be intermediate 3rd-party wrapper packages in the mix, meaning that it isn't even possible to simply rebuild it - you need to petition the 3rd party (who might be busy or otherwise unavailable) to update and redeploy their package - and there might even be multiple chained 3rd parties that need to update in the right order!

So, yeah. We cannot and will not just add optional parameters: instead, we need to add a new overload with the additional parameter. Note, however, that where appropriate it is possible to retire old overloads by simply removing the "this" modifier; this is not a binary break, and is not a source break as long as the remaining overloads over the existing and new use-cases. Most of the time, however, both the old and new overloads retain the "this".

Hi, I'm a bit unsure why this would be considered a breaking change as the new CancellationToken parameter is optional when calling the QueryAsync method.

I hope the above explains why this is a breaking change as written.

That said I'm new to this code base, perhaps I'm missing something?

Nothing in the above is specific to this codebase - it is fundamentally how library packages work. This is less of a problem if the only people who consume a library are in the same team, and everything will always get updated together: but if you're a public library, this is not the case.

However, if this is really a breaking change, then I agree that it would make most sense to wait with merging this until the next major release.

Even at majors, we don't wilfully make breaking changes without a very good reason. In this case, the change can be applied by adding overloads rather than parameters.

[mhov]

After using Dapper for years and years I finally needed to integrate cancellation tokens to discover it's a hot topic. When's Dapper 3.x?

[WolfyUK]

Could this be achieved with a CommandDefinition overload instead, then? e.g. (in SqlMapper.Async.cs):

public static Task<IEnumerable<TReturn>> QueryAsync<TReturn>(this IDbConnection cnn, CommandDefinition command, Type[] types, Func<object[], TReturn> map, string splitOn = "Id")
{
   return MultiMapAsync(cnn, command, types, map, splitOn);
}

[lassevk]

I was going to ask for something similar for ExecuteAsync, which also lacks the CancellationToken parameter. Would a PR with an additional overload, not touching the existing one(s), be more welcome for that?

ExecuteAsync with parameters simply construct a CommandDefinition behind the scenes, with a default value for the cancellation token, it would be a simple matter to add an additional overload with this parameter lifted.

[0xced]

I tried to implement what @mgravell suggested.

So, yeah. We cannot and will not just add optional parameters: instead, we need to add a new overload with the additional parameter. Note, however, that where appropriate it is possible to retire old overloads by simply removing the "this" modifier.

It looks like it could work but I'm still currently fighting with the public API analyzers. Currently using <NoWarn>RS0016;RS0017</NoWarn> to get the project to compile. 😅

I think I should be able to open a new pull request soon to add natural CancellationToken support to all Dapper methods.

Edit: I just pushed https://github.com/0xced/Dapper/tree/CancellationToken if anyone is interested.

[0xced]

I have followed Marc's advice for preserving binary and source compatibility. I just opened #2178 which should supersede this pull request.