feat(async/unstable): support allowInfinity option in deadline

[lowlighter]

This add a new allowInfinity?: boolean; option to deadline to allow Infinity value.

import { parseArgs } from "@std/cli/parse-args";
const { timeout = Infinity } = parseArgs(Deno.args);

await deadline((async () => {
  // Long running task
})(), timeout, { allowInfinity: true })

Why is this needed ?

• deadline/abortable doesn't have the same signature, so you can just interchange them inline
  • Or else you need to be verbose and put out your promise, checking the timeout value and decide to call or not deadline
• AbortSignal.timeout() only support finite values, so using Infinity would throw an error (current behaviour)
• An Infinity deadline makes senses concept-wise, it's just an edge case
• This simplify a lot apps where end-users can customize the timeout themselves (like in the example above)

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.23%. Comparing base (42fd485) to head (68c72b8).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #6810   +/-   ##
=======================================
  Coverage   94.23%   94.23%           
=======================================
  Files         590      591    +1     
  Lines       42811    42823   +12     
  Branches     6788     6790    +2     
=======================================
+ Hits        40341    40354   +13     
+ Misses       2420     2419    -1     
  Partials       50       50           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[lionel-rowe]

If this adds support for Infinity, why have an additional option for allowInfinity? Why not just always allow it?

Also, should Infinity be a special case, or should this just add support for arbitrarily large values? See #6775 for an example of how that could work (could move setArbitraryLengthTimeout into a common internal utils file, and/or create a corresponding arbitraryLengthAbortSignalTimeout class).

[lowlighter]

I added it as an option because it could be considered a breaking change, and some people might expect the deadline to eventually be resolved in a finite amount of time. But personally I think it would make sense to just support it without the option

As for the other question, I guess this could be iterated in another PR. But since the spec of AbortSignal.timeout only accepts until Number.MAX_SAFE_INTEGER, it might be tedious to implement maybe ?

[lionel-rowe]

I added it as an option because it could be considered a breaking change, and some people might expect the deadline to eventually be resolved in a finite amount of time.

Changing throwing to non-throwing behavior typically isn't considered breaking (edit: I guess (?) that also means this wouldn't need to be marked unstable, as the interface wouldn't need to change at all).

As for the other question, I guess this could be iterated in another PR.

It's true the actual use case for using values in 0x1fffffffffffff < n < Infinity is probably pretty limited, but disallowing numbers within that range while specifically allowing Infinity seems like a surprising limitation to me.

But since the spec of AbortSignal.timeout only accepts until Number.MAX_SAFE_INTEGER, it might be tedious to implement maybe?

Using similar logic to setArbitraryLengthTimeout:~~

const MAX_ABORT_TIMEOUT = Number.MAX_SAFE_INTEGER

function arbitraryLengthAbortSignalTimeout(delay: number): AbortSignal {
  const ac = new AbortController();
  // ensure non-negative integer (but > MAX_ABORT_TIMEOUT is OK, even if Infinity)
  let currentDelay = delay = Math.trunc(Math.max(delay, 0) || 0);
  const start = Date.now();

  const queueTimeout = () => {
    currentDelay = delay - (Date.now() - start);

    if (currentDelay > MAX_ABORT_TIMEOUT) {
      AbortSignal.timeout(MAX_ABORT_TIMEOUT).onabort = queueTimeout;
    } else {
      AbortSignal.timeout(Math.max(0, currentDelay)).onabort = function() {
        ac.abort(this.reason);
      };
    }
  };

  queueTimeout();
  return ac.signal;
}

[lowlighter]

It's true the actual use case for using values in 0x1fffffffffffff < n < Infinity is probably pretty limited, but disallowing numbers within that range while specifically allowing Infinity seems like a surprising limitation to me.

I mean it depends.

Most CLI apps that supports a timeout option usually accepts specifying passing either 0/-1 as special values to disable timeout completly (e.g. wget). But using these values for deadline() might seems counter-intuitive. Maybe this PR should instead be an option like zeroNoDeadline (not sure about the naming)

Infinity makes sense semantically but as you said it might get confusing for in-between range.

But then maybe it's actually useless to do, I mean, I currrently use Math.min(Number.MAX_SAFE_INTEGER, timeout) as a workaround and it's already like 285 616 years... We're probably all be dead by that time 😅

[lionel-rowe]

But then maybe it's actually useless to do, I mean, I currently use Math.min(Number.MAX_SAFE_INTEGER, timeout) as a workaround and it's already like 285 616 years... We're probably all be dead by that time 😅

Oh yeah, I guess implementing this would be thoroughly pointless haha. The equivalent max value for setTimeout is only 0xffffffff i.e. ~50 days, which is still conceivably within the lifespan of a long-running program.

IMO either one of Math.min(Number.MAX_SAFE_INTEGER, ms) or just returning new AbortController().signal (and never aborting it) above Number.MAX_SAFE_INTEGER would be equally good. Either case avoids special-casing Infinity, and either case would never (practically speaking) lead to wrong behavior.

[kt3k]

@lowlighter sorry for the delay in review! I like @lionel-rowe's suggestion. Never aborting when the timeout > Number.MAX_SAFE_INTEGER makes sense to me.

I also think this can be landed as fix without introducing new option (I think we can just change (fix) the behavior for large ms value). Can you just change the existing deadline?