louthy / language-ext

C# functional language extensions - a base class library for functional programming
MIT License
6.48k stars 417 forks source link

Prelude timeout does not use the HasTime<RT> runtime trait #1012

Open timmi-on-rails opened 2 years ago

timmi-on-rails commented 2 years ago

I was surprised that the runtime-dependent timeout implementation does not utilize the HasTime<RT> trait for the internal timeout task.

Current version

// signature (see https://github.com/louthy/language-ext/blob/main/LanguageExt.Core/Effects/Aff/Prelude/Aff.Prelude.cs#L228)
public static Aff<RT, A> timeout<RT, A>(TimeSpan timeoutDelay, Aff<RT, A> ma) where RT : struct, HasCancel<RT>

// timeout task implementation (see https://github.com/louthy/language-ext/blob/main/LanguageExt.Core/Effects/Aff/Aff.cs#L174)
var delay = Task.Delay(timeoutDelay, delayTokSrc.Token);

What I expected

// signature
public static Aff<RT, A> timeout<RT, A>(TimeSpan timeoutDelay, Aff<RT, A> ma) where RT : struct, HasCancel<RT>, HasTime<RT>

// timeout task implementation
// something like...
var delay = ?...? env.TimeEff.Map(timeIO => timeIO.SleepFor(timeoutDelay, delayTokSrc.Token)) ?...?

Is my expectation unusual?

(If not, any change is probably a breaking change, so it comes down to extending the method documentation and maybe provide a timeout2 function)

louthy commented 2 years ago

@timmi-on-rails I don't think it's unreasonable to think that, however, I don't think it adds much in the way of tangible benefits here. A couple of reasons I think this:

There could be an argument, which I think is fair, that by making this use HasTime for its delay-task, we could allow time to slow down or pause. That could certainly have some benefits, although even that would need some very careful coding of the timeout behaviour ... not impossible, but of debateable value.

timmi-on-rails commented 2 years ago

@louthy I fully agree to your second point, but only partially to your first point. While there is only one valid way to timeout in a Live Environment, there are still alternative valid implementations how to timeout in a test environment. Here is some context:

I have written a chat bot, which uses timeouts when waiting for user responses and then sends a reminder to answer the bot's message. Now I would like to write some tests. My plan was to time-travel with the HasTime Test Runtime implementation and speed up test durations. This way I don't have to wait 5 minutes for the timeout until the test passes.

louthy commented 2 years ago

@timmi-on-rails That's a reasonable request, let me have a think 👍