-
Notifications
You must be signed in to change notification settings - Fork 300
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
Timeout AbortSignal (or AbortController) that gets input from the operation #1082
Comments
How I think of this: Our base primitive is We then add helpers for common cases where we can achieve a lot of simplicity. So const controller = AbortController();
setTimeout(() => controller.abort(new DOMException("TimeoutError", "..."), ms);
return controller.signal; to just The compression here would probably be something like: const controller = AbortController();
let timerId = setTimeout(() => controller.abort(new DOMException("TimeoutError", "..."), ms);
onUpdate(() => {
clearTimeout(timerId);
timerId = setTimeout(() => controller.abort(new DOMException("TimeoutError", "..."), ms);
});
return controller.signal; to const { signal, reset } = createHelper(ms);
onUpdate(() => reset());
return signal; I'm not sure this kind of compression is worth it, but maybe it is. If it were added, I think it could be separate from any Fetch addition. I.e., any Fetch addition would probably build on top of such a helper, but not in a web-developer-visible way. Because the |
I think having a simple class TimeoutAbortController extends AbortController { /* ... */ }
auto controller = new TimeoutAbortController(10 * 1000);
useSignalSomewhere(controller.signal);
// Call reset() to reset the timer.
controller.reset();
// TimeoutAbortController extends from AbortController, so still possible to call abort() directly.
controller.abort(); This would simplify some of the cases motivating the |
Thinking about this more. I think what I'd like to see here are several additional APIs for both
I would go with either all three or only the last two. Examples: const signal = AbortSignal.timeout(10_000);
// ...
signal.reset(); // resets the timer to 10_000
//
signal.cancel(); // clears the timer, the signal will never trigger const ac = new AbortController();
const signals = AbortSignal.any([ac.signal, AbortSignal.timeout(10_000)]);
ac.cancel();
// signals will only follow the timeout signal now
signals.reset();
// has no impact on the timeout signal if it hasn't already been triggered.
// signals will still trigger once the timeout completes.
// if signals has already been triggered, and both of the followed signals
// have either been canceled or triggered already, then signals will never
// trigger again after reset. |
I think it is very important for the signal to be unable to control upstream effects, such as remaining time. One of the motivations for timed AbortSignals is deadlines: "all work must complete in this given time, any response past then will be ignored". If a receiver of a signal is able to simply reset a deadline then it severely limits the efficacy of passing such a value. |
Yeah, I think @keithamus is correct. For the scenarios in OP Fetch will have to own some kind of timer itself. There could also be a signal that is passed in (that ultimately wins) and the functionality might be implemented in terms of an internal signal Fetch controls, but the features seem kinda separate from the way signals normally work. Adding more functionality to the controller seems reasonable, but I think we're already tracking that in other issues. |
I feel like there are cases where you want feedback to affect related operations. Give up on a set of operations if nothing returns in time, otherwise they're still relevant so keep them all running. const signal = AbortSignal.timeout(5000, { cancelable: true })
// Calculations detect a cancelable signal,
// cancel to keep the whole set running on first returned value
const stream = new CombineStream([
generateNumbers(startFrom, Type.PrimeNumbers, { signal }),
generateNumbers(startFrom, Type.SurrealNumbers, { signal }),
generateNumbers(startFrom, Type.VampireNumbers, { signal })
]) Run 3 requests to the same server and keep them all alive as long as anything is returning data. const signal = AbortSignal.timeout(5000, { resettable: true })
// fetch detects a resettable signal,
// and keeps it refreshed from all requests
const datasets = Promise.all([
fetch('/WesternRegion.json', { signal })
.then(response => response.json()),
fetch('/EasternRegion.json', { signal })
.then(response => response.json()),
fetch('/SouthernRegion.json', { signal })
.then(response => response.json())
]) It's really important not to make signals consumer-mutable by default. But where the operations are related, and you can guarantee the signal will never go to anything else, this is something I could use. |
Both whatwg/fetch#179 and whatwg/fetch#180 explain a desire for a timeout, but either the countdown gets reset based on networking activity (new chunk received) or it gets cancelled based on that (server was responsive within the allowed time).
Is that something
fetch()
should offer on its own or is there some decomposition of responsibility that could make sense here, wherebyfetch()
gets access to resetting or cancelling the timer.The text was updated successfully, but these errors were encountered: