BREAKING(async): simplify deadline() logic, remove DeadlineError and improve errors

[iuioiua]

What's changed

This PR:

1. Ensures that all thrown/rejected errors within @std/async APIs are DOMExceptions, with the appropriate AbortError or TimeoutError error names.
2. Simplifies deadline() by relying on abortable().
3. Removes DeadlineError for deadline(), as it's no longer needed.
4. Clarifies the error message when one of these functions is called and the AbortSignal is already aborted.

Why was this change been made?

These changes were made to:

1. Ensure this package's APIs are consistent in how they throw errors.
2. Reduce the number of moving parts.
3. Reduce the need to import an additional custom error, DeadlineError.
4. Improve the clarity of errors and therefore help with troubleshooting.

Migration guide

If using deadline() compare the error with DOMException instead of DeadlineError when handling errors.

- import { deadline, type DeadlineError } from "@std/async";
+ import { deadline } from "@std/async";

  try {
    await deadline(promise, 1_000);
  } catch (error) {
-   if (error instanceof DeadlineError && error.message === "Deadline") {
+   if (error instanceof DOMException && error.name === "TimeoutError") {
      // Handle error
    }
  }

Related

Depends on denoland/deno#23842

[kt3k]

The main consequences of these changes are that the error type has changed from DeadlineError to DOMException and the error message from Deadline to The signal has been aborted. Deadline error has been removed as it's no longer used.

I don't understand why we want to make this change. Currently the user can distinguish whether the promise is aborted by signal or aborted by deadline by catching the error. Now they can't do it.

[iuioiua]

You can compare using error.name === "AbortError" or error.name === "TimeoutError", similar to AbortSignal.timeout(). I've clarified my initial comment. Perhaps, we can update the documentation as well.

[kt3k]

You can compare using error.name === "AbortError" or error.name === "TimeoutError", similar to AbortSignal.timeout().

Oh, this sounds nice.

I've clarified my initial comment. Perhaps, we can update the documentation as well.

Sounds a good idea. Can you update the doc examples and test case to demonstrate this?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 92.70%. Comparing base (15ec8df) to head (fbab337).
Report is 3 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #5058   +/-   ##
=======================================
  Coverage   92.70%   92.70%           
=======================================
  Files         482      482           
  Lines       38713    38723   +10     
  Branches     5462     5457    -5     
=======================================
+ Hits        35888    35898   +10     
  Misses       2770     2770           
  Partials       55       55           

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

[iuioiua]

I've decided to revert the use of signal.throwIfAborted(), as it throws the reason, which is a string, instead of an error with the message as the reason.

[iuioiua]

I've tweaked the direction of this PR and re-written the descript. @kt3k, please take a look once more.

[0f-0b]

I've decided to revert the use of signal.throwIfAborted(), as it throws the reason, which is a string, instead of an error with the message as the reason.

I think it's better to always just throw the reason itself, to be consistent with Web APIs like fetch and navigator.locks.request.

[iuioiua]

I think it's better to always just throw the reason itself, to be consistent with Web APIs like fetch and navigator.locks.request.

This code does, wrapped in a DOMException. Do you see any spots where it doesn't?

[0f-0b]

This code does, wrapped in a DOMException. Do you see any spots where it doesn't?

Here the reason (which is usually already an "AbortError" DOMException) is implicitly converted to a string and used as the message of another exception. That is no longer the reason itself.

[kt3k]

Maybe ac.abort() method suppose the reason should be an error object instead of string?

[iuioiua]

Oh, I see. Thank you. I've made some adjustments, which made things even cleaner. PTAL.

[iuioiua]

On 2nd thought. Maybe this should throw. Thoughts?

[kt3k]

This looks fine to me

[kt3k]

LGTM

I think the removal of wrapping of abort reason is a good move towards Web standard alignment. Thanks @0f-0b for the ideas!