-
Notifications
You must be signed in to change notification settings - Fork 298
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce AbortSignal.timeout() #1032
Conversation
Ah, yeah it's an oversight if |
Yeah, in that case I will work on a HTML spec PR pulling out the common infrastructure. For now it will behave exactly like the shared parts of postTask/setTimeout. If we decide AbortSignal.timeout() should behave the same as those with regard to bfcache/suspended workers then this PR can just use that; otherwise we can add a flag. |
This factors out a new algorithm which can be used by postTask() (https://wicg.github.io/scheduling-apis/#schedule-a-posttask-task) and AbortSignal.timeout() (whatwg/dom#1032), ensuring that they correctly contribute to idle deadline computation and in general share all the appropriate logic with setTimeout() and setInterval(). This also exports the "timer task source" term since AbortSignal.abort() will want to use that.
I did the centralization work at whatwg/html#7349, and sent WICG/scheduling-apis#54 plus updated this PR to match. Now this PR does pause the timeout while in bfcache/while the worker is suspended. We should discuss whether that's a good idea or not; it's certainly the path of least resistance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like it! And yeah, Mozilla can be considered supportive.
Refs: whatwg/dom#1032 Signed-off-by: James M Snell <jasnell@gmail.com>
Refs: whatwg/dom#1032 Signed-off-by: James M Snell <jasnell@gmail.com>
Refs: whatwg/dom#1032 Signed-off-by: James M Snell <jasnell@gmail.com>
This factors out a new algorithm which can be used by postTask() (https://wicg.github.io/scheduling-apis/#schedule-a-posttask-task) and AbortSignal.timeout() (whatwg/dom#1032), ensuring that they correctly contribute to idle deadline computation and in general share all the appropriate logic with setTimeout() and setInterval(). This also exports the "timer task source" term since AbortSignal.abort() will want to use that.
I was going to write a big comment with the pros/cons of suspending while in bfcache/in a worker. But in the end it mostly ended up with pros for suspended. So I am currently advocating we keep the Here's was my thought process on the pros/cons of suspending:
@shaseley, does this reasoning seem reasonable to you? I'd like to have your blessing on our decision here, as the person who has thought most about these sorts of timing and scheduling things in Chromium :) |
Yep, this sounds reasonable; suspending in bfcache LGTM. These roughly match what I've been thinking, and great point about how
Note: this is unfortunately broken in in Chromium, but I do like the consistency.
Yeah aborting stuff on restore would be good in this scenario, although timer storms could still result if these signals have
We're recommending that devs close connections in Combining a timeout signal with one that gets aborted on
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main thing I'm worried about for Firefox is that I think @farre or @smaug---- at some point mentioned to me that the clamping logic sits pretty deep in our timeout code. And beyond that, I worry that if we don't enforce clamping for these new timer mechanisms we'll eventually end up back there due to misbehaving websites. But perhaps if no browser budges it's feasible to hold the door.
Refs: whatwg/dom#1032 Signed-off-by: James M Snell <jasnell@gmail.com>
You can add Node.js and Cloudflare Workers to the list of runtime implementations in support of this. I have PRs pending to land in both environments implementing the |
This factors out a new algorithm which can be used by postTask() (https://wicg.github.io/scheduling-apis/#schedule-a-posttask-task) and AbortSignal.timeout() (whatwg/dom#1032), ensuring that they correctly contribute to idle deadline computation and in general share all the appropriate logic with setTimeout() and setInterval(). This also exports the "timer task source" term since AbortSignal.abort() will want to use that.
I feel like we (Mozilla and Chromium) discussed this in the context of postTask() and our conclusion was that the allowance for implementation-defined waiting was enough, at least to start with. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, that makes sense.
Alright, I've updated this with a mini-explainer. All that remains are someone to write some web platform tests, probably as part of an implementation :) |
So the signal on the fetch (in the example) would be aborted even if the fetch() succeeds, abort would just happen when the timeout happens to run? Isn't that error prone, getting abort event, even though the fetch() itself has succeeded? |
I'm not sure I understand the concern. Nobody else is listening for the abort event in the example... and if they were, then the contract is that the AbortSignal should signal abort to both the fetch (which ignores it) and that other thing (which doesn't), so it seems like it's working as intended. |
The concern, which may not be that major, is just that one gets now always an aborted signal eventually, so the event listener on the signal is called, even though fetch() or other APIs using signal succeed. |
I think the issue is more that the timer can continue well after the set of operations completed successfully. Take a pathological case where I create an const signal = AbortSignal.timeout(1_200_000);
signal.addEventListener('abort', () => {
throw signal.reason;
}); The complete set of operations that I pass the Ideally there would be a way of canceling the timeout. Obviously that doesn't help with the closure holding on to things but it ensures that the underlying timer it at least no holding the reference for way longer than it needs to. const signal = AbortSignal.timeout(1_200_000);
await fetch('http://example.org', { signal });
signal.clearTimeout(); This case is similar to why we allowed adding an const cancelAbort = AbortController();
const signal = AbortSignal.timeout(1_200_000, { signal: cancelAbort.signal });
signal.addEventListener('abort', () => {
throw signal.reason;
}, { signal: cancelAbort.signal });
await fetch('http://example.org', { signal });
cancelAbort.abort(); Granted, this doesn't prevent the pathological case but it provides a Right Way To Do It alternative. |
I'm still having trouble understanding the concern. Could someone produce a semi-realistic example of what someone might do with the current proposal, and how they think that the code would give unexpected results? |
Is this example not realistic? const signal = AbortSignal.timeout(1_200_000);
signal.addEventListener('abort', () => { throw signal.reason; });
await fetch('http://example.org', { signal }); |
Well, I'm not sure what the consumer on the second line is trying to do. What about that example is not working as expected? I.e. what are the expected results, vs. the actual results in this proposal? |
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9
Refs: whatwg/dom#1032 Signed-off-by: James M Snell <jasnell@gmail.com> PR-URL: #40899 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406}
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406}
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406}
Tests have landed! Merging, and will file browser bugs and edit the OP with them momentarily. |
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406}
Automatic update from web-platform-tests Implement AbortSignal.timeout AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406} -- wpt-commits: 2b811aa5c2695cb3fd6f895cdce5952d1e0dd22d wpt-pr: 32622
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406}
Automatic update from web-platform-tests Implement AbortSignal.timeout AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406} -- wpt-commits: 2b811aa5c2695cb3fd6f895cdce5952d1e0dd22d wpt-pr: 32622
Automatic update from web-platform-tests Implement AbortSignal.timeout AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406} -- wpt-commits: 2b811aa5c2695cb3fd6f895cdce5952d1e0dd22d wpt-pr: 32622
Automatic update from web-platform-tests Implement AbortSignal.timeout AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406} -- wpt-commits: 2b811aa5c2695cb3fd6f895cdce5952d1e0dd22d wpt-pr: 32622
This factors out a new algorithm which can be used by postTask() (https://wicg.github.io/scheduling-apis/#schedule-a-posttask-task) and AbortSignal.timeout() (whatwg/dom#1032), ensuring that they correctly contribute to idle deadline computation and in general share all the appropriate logic with setTimeout() and setInterval(). This also exports the "timer task source" term since AbortSignal.abort() will want to use that.
AbortSignal.timeout is a static method that creates a new AbortSignal that is automatically aborted after a specified duration. The implementation is essentially PostDelayedTask(SignalAbort, ms). Throttling: this API is specced to use the timer task source, but there are three internally due to our throttling implementation. We use immediate for the timeout == 0 case and high nesting for timeount > 0 (the typical case), i.e. all non-zero timeouts are eligible for throttling (Note: this matches scheduler.postTask()). Spec PR: whatwg/dom#1032 I2P: https://groups.google.com/a/chromium.org/g/blink-dev/c/9Y290P1WimY/m/bru989iAAgAJ Bug: 1181925 Change-Id: I192d82a8bf12c368abcd47ae6c50e80f50654cf9 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3425124 Reviewed-by: Domenic Denicola <domenic@chromium.org> Reviewed-by: Mason Freed <masonf@chromium.org> Commit-Queue: Scott Haseley <shaseley@chromium.org> Cr-Commit-Position: refs/heads/main@{#966406} NOKEYCHECK=True GitOrigin-RevId: 6418fb5de0252c4cca60b9027237ad24f074465a
Closes #951.
Mini-explainer for TAG review/blink-dev purposes:
This introduces a new static method,
AbortSignal.timeout(milliseconds)
, which returns a newly-createdAbortSignal
that, aftermilliseconds
milliseconds, becomes automatically aborted. TheAbortSignal
'sreason
property will be a"TimeoutError"
DOMException
.The main motivating use case for this is helping web developers easily time out async operations, such as
fetch()
. For example, now you can write:to ensure your fetch operation has a 10-second timeout. A more full example would be:
This has come up in the context of several specs; see discussions on fetch at whatwg/fetch#951 and streams at WICG/serial#122 (comment). It is generally helpful for any operation that takes an
AbortSignal
, including web-developer-defined operations.Interesting points of discussion are:
It is specifically important that the reason be a
"TimeoutError"
DOMException
, instead of the default-for-AbortSignal
"AbortError"
DOMException
. This is because developers almost always want to discriminate between those two: a timeout is a proper failure which they might show to users, whereas an abort usually more of a "third state" (alongside success and failure) which usually they don't want to tell users about. (E.g., because the user is the one that caused the abort in the first place.)We treat the
milliseconds
argument the same asscheduler.postTask()
, and almost the same assetTimeout()
. (The difference is, we don't have the nested-clamping-to-4-ms behavior thatsetTimeout()
does.) In particular this means that the timer does not advance while the document is in bfcache, or the worker is suspended. See Introduce AbortSignal.timeout() #1032 (comment) for the reasoning there.Future work: this feature would benefit greatly from a solution for combining
AbortSignal
s, so that people could have an operation that aborts on either a timeout or an explicit abort. For example, something like this:Unfortunately there are a lot of possible shapes for such an API and so we still need to do a bit more work and research to find the best one. That's tracked in #920. Until then, people can combine such signals manually, as discussed in that issue thread.
(See WHATWG Working Mode: Changes for more details.)
Now-obsolete discussion of "Subtle things to deal with"
Preview | Diff
Preview | Diff