Re: [whatwg/dom] Add abort reason to AbortSignal (PR #1027)

@domenic requested changes on this pull request.

> With setting nits aside, I have a question - when no reason is given to AbortController.abort(), should AbortController.reason be undefined, or an AbortError? I think undefined is simpler and I prefer the option if there is no reason to create a new AbortError.

I think it's important to create a new AbortError so that author code can do something like

```js
async function doSomething({ signal }) {
  if (signal.aborted) {
    throw signal.reason;
  }
}
```

to propagate the error as expected. This is also how I would expect us to write the other specs, e.g. Fetch would say something like "If _signal_ is aborted, then reject _promise_ with _signal_'s abort reason."

> @@ -1729,7 +1729,7 @@ interface AbortController {
 
   [SameObject] readonly attribute AbortSignal signal;
 
-  undefined abort();
+  undefined abort(optional any reason = null);

Let's remove `= null`; we can just ask if the argument is present or not.

> @@ -1762,8 +1763,10 @@ constructor steps are:
 <p>The <dfn attribute for=AbortController><code>signal</code></dfn> getter steps are to return
 <a>this</a>'s <a for=AbortController>signal</a>.
 
-<p>The <dfn method for=AbortController><code>abort()</code></dfn> method steps are to
-<a for=AbortSignal>signal abort</a> on <a>this</a>'s <a for=AbortController>signal</a>.
+<p>The <dfn method for=AbortController><code>abort(reason)</code></dfn> method steps are to
+<a for=AbortSignal>signal abort</a> on <a>this</a>'s <a for=AbortController>signal</a>, with
+the provided <var>reason</var>. If a <var>reason</var> is not passed, create a new "{{AbortError!!exception}}"
+{{DOMException}}.

"Signal abort" does not accept a second reason argument, so this is invalid.

I think actually the defaulting logic should take place in the "signal abort" algorithm.

Also we need to update other call sites of "signal abort", e.g. in "follow".

> @@ -1790,6 +1795,9 @@ interface AbortSignal : EventTarget {
 <p>An {{AbortSignal}} object has an associated <dfn export for=AbortSignal>aborted flag</dfn>. It is

You should add a dt/dd pair to the `<dl class=domintro>` above this, giving a web-developer-facing explanation of the `reason` property.

> @@ -1790,6 +1795,9 @@ interface AbortSignal : EventTarget {
 <p>An {{AbortSignal}} object has an associated <dfn export for=AbortSignal>aborted flag</dfn>. It is
 unset unless specified otherwise.
 
+<p>An {{AbortSignal}} object has an associated <dfn export for=AbortSignal>abort reason</dfn>, which is an
+argument given to {{AbortController}}'s {{AbortController/abort()}}, or undefined.

I would say "is a JavaScript value".

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/whatwg/dom/pull/1027#pullrequestreview-782289936