How to improve my code where I need to undo what I did

[shoter]

Here is the code :

  public EitherAsync<IError, StatusMonitor> Execute(User user, OttdServer server, ulong guildId, ulong channelId)
        {
            var embedMessageIdResult = CreateEmbedMessage(channelId, server);

            return
            (
             from _1 in CheckIfHasCorrectUserLevel(user, UserLevel.Admin).ToAsync()
             from messageId in embedMessageIdResult
             from statusMonitor in statusMonitorRepository.Insert(new StatusMonitor(
                 server.Id,
                 guildId,
                 channelId,
                 messageId,
                 DateTime.MinValue.ToUniversalTime()))
             from _2 in InformActor(statusMonitor)
             select statusMonitor)
            .MapLeft(err =>
            {
                if (embedMessageIdResult.IsRight.Result)
                {
                    DeleteEmbedMessage(channelId, embedMessageIdResult.Right());
                }

                return err;
            });
        }

I am new to this library and I am wondering if there is any nicer approach to handle errors in this case to call DeleteEmbedMessage if any message was created and if something failed afterwards.

[StefanBertels]

Very interesting question. I'm interested in nice solutions for this, too.

Finally this is similar to database transactions where you can rollback (or commit). IMHO code gets more functional (FP style) if we try to avoid any intermediate side effect. But I find this is quite often impossible. An easy example is "create 2 files" which might fail after file 1 is created.

Maybe something like this:

public sealed class TransactionLog : IDisposable
{
    private readonly Stack<IDisposable> _rollbacks = new ();

    public Unit AddRollback(Action rollbackAction)
    {
        _rollbacks.Push(Disposable.Create(rollbackAction));
        return unit;
    }

    public Unit Commit()
    {
        _rollbacks.Clear();
        return unit;
    }

    public void Dispose()
    {
        while (_rollbacks.TryPop(out var disposable))
        {
            disposable.Dispose();
        }
    }
}

// example test with code writing content to two files where writing file2 might fail
[Theory]
[InlineData(true)]
[InlineData(false)]
void Transaction(bool file2Ok)
{
    var fn1 = Path.GetTempFileName();
    var fn2 = Path.GetTempFileName();

    var file1Written = Option<bool>.None;

    string WriteAllText(string path, string content)
    {
        File.WriteAllText(path, content);
        return path;
    }

    try
    {
        var result = use(new TransactionLog(), transaction =>
            from file1PathWritten in Try(() => WriteAllText(fn1, "lang-ext ")).ToEither()
            let file1Rollback = transaction.AddRollback(() => WriteAllText(file1PathWritten, ""))
            let _ = file1Written = new FileInfo(fn1).Length > 0
            from file2PathWritten in Try(() => !file2Ok ? failwith<string>("test") : WriteAllText(fn2, "rocks!")).ToEither()
            let file2Rollback = transaction.AddRollback(() => WriteAllText(file2PathWritten, ""))
            let ok = transaction.Commit()
            select "committed");

        Assert.Equal(true, file1Written);
        Assert.Equal(new FileInfo(fn1).Length == 0, new FileInfo(fn2).Length == 0); // both empty or both filled

        if (file2Ok)
        {
            Assert.Equal("committed", result);
            Assert.Equal("lang-ext rocks!", File.ReadAllText(fn1) + File.ReadAllText(fn2));
        }
        else
        {
            Assert.True(result.IsLeft, result.ToString());
            Assert.Equal("", File.ReadAllText(fn1) + File.ReadAllText(fn2));
        }
    }
    finally
    {
        File.Delete(fn1);
        File.Delete(fn2);
    }
}

Note: edited because code was not working / improved test showing both cases.

[shoter]

This is interesting Idea. I will try to implement something like this.

[shoter]

My code now looks as follow and you can see it here, if someone is interested:

        public EitherAsync<IError, StatusMonitor> Execute(User user, OttdServer server, ulong guildId, ulong channelId)
        {
            var embedMessageIdResult = CreateEmbedMessage(channelId, server);
            var transactionLog = new TransactionLog();
            return
            (
             from _1 in CheckIfHasCorrectUserLevel(user, UserLevel.Admin).ToAsync()
             from messageId in embedMessageIdResult
             from _2 in transactionLog.AddTransactionRollback(() => DeleteEmbedMessage(channelId, messageId)).ToAsync()
             from statusMonitor in statusMonitorRepository.Insert(new StatusMonitor(

                 server.Id,
                 guildId,
                 channelId,
                 messageId,
                 DateTime.MinValue.ToUniversalTime()))
             from _4 in InformActor(statusMonitor)
             select statusMonitor)
             .LeftRollback(transactionLog);
        }

[StefanBertels]

Looks good: Your code looks much more cleaner and I take the chance to improve my answer a little bit, just in case someone else uses it...
:-)

One idea why I personally would use IDisposable resp. using/use here:

If anything goes wrong (Exception or other early return) and some explicit 'LeftRollback' does not get called having some using will make rollback work in that cases (since it is called always in finally). Of course the drawback is that you need to Commit explicitly. But IMHO it is easier to avoid errors in the good path (you will detect them earlier / basic unit tests should cover them) than in some bad path (exhaustive unit tests?). Just a note.

PS: Another problem is, how to handle errors in rollback actions. Silently ignore might be a good fit for my IDisposable solution. But again this might hide bugs. What about multiple rollback actions? Do you want to continue on errors? Glad to hear some ideas...