Race condition in document visibility checking can result in duplicate requests being made #1096
Description
Bug Report
To reproduce, have two pages:
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<title>data-init test</title>
</head>
<body>
<a href="/index2.html">Click me</a>
</body>
</html>and
<!doctype html>
<html lang="en">
<head>
<meta charset="UTF-8" />
<title>data-init page 2</title>
<script type="module" src="/datastar.js"></script>
</head>
<body>
<main
data-init="@get('/api/updates')"
>
</main>
</body>
</html>Load the first page and then command/ctrl-click on the link to open the second page in a new tab.
What happens
When the page is loaded, the @get() request is made to the backend. When the user switches to the tab, the visibilitychange event fires. The initial request is aborted and a new request is created. The intent is that a single request remains active to the backend. However, due to the race condition, the page ends up creating two requests.
Cause
An AbortController is used to abort the requests, which is created when the request is created. However, when the first request is aborted, a new one is created immediately. The result is that the instance of the AbortController is replaced by a new one which has not been aborted. The initial request will then throw an AbortError, which is caught. A check is then performed on the AbortController's signal to see whether the request had been aborted. If it had not, then the request is restarted. Because the AbortController instance has been replaced, it will look like the initial request had not been aborted, and so it will be recreated, resulting in two in-flight requests.
Fix
The following fix closes over the signal rather than the AbortController and therefore means the correct signal is checked when the AbortError is thrown:
diff --git a/library/src/plugins/actions/fetch.ts b/library/src/plugins/actions/fetch.ts
index a74ccbae48..45cc7104bc 100644
--- a/library/src/plugins/actions/fetch.ts
+++ b/library/src/plugins/actions/fetch.ts
@@ -612,11 +612,12 @@
let baseRetryInterval = retryInterval
const create = async () => {
curRequestController = new AbortController()
+ const signal = curRequestController.signal
try {
const response = await fetch(input, {
...rest,
headers,
- signal: curRequestController.signal,
+ signal: signal,
})
// on successful connection, reset the retry logic
@@ -716,7 +717,7 @@
dispose()
resolve()
} catch (err) {
- if (!curRequestController.signal.aborted) {
+ if (!signal.aborted) {
// if we haven’t aborted the request ourselves:
try {
// check if we need to retry:Datastar Version
RC6