Repository navigation
Description
Versions
- Client: typescript,
@electric-sql/client1.5.28 (also reproduced on 1.5.24 and 1.5.26; the same lines are onmainat3f9809fb0) - Electric: not involved. The repro below uses a scripted
fetchClientand no server.
Bug description
If a ShapeStream pause is released in the same task that acquired it, the stream can stop for good with:
TypeError: Cannot read properties of undefined (reading 'signal')
We hit this in production browser tabs. A runtimeVisibility adapter sends hidden then visible when a tab returns to the foreground, to abort a request the browser may have suspended and resume with a catch-up request. When onError has no retry to offer, the stream ends silently and the collection above it keeps serving stale rows.
Expected behavior
The aborted request unwinds and the resumed request carries on, however quickly the pause is released.
Cause
Two request loops overlap and share one field:
- Acquiring the pause aborts the shared controller. Releasing it calls
#start()straight away, while the aborted request is still unwinding. - The new request sets
#requestAbortControllerinside#createAbortListener, yields on thatawait, and then reads the field back with a non-null assertion. - The old request's
finallyclears the field unconditionally. If that runs inside the yield, the new request readsundefined.
The error then comes from the catch around the fetch, which reads requestAbortController.signal.reason.
Whether the two interleave depends on how many promise jobs the abort takes to unwind and how long header resolution takes, so it is timing-sensitive. Real fetch wrappers such as error-monitoring instrumentation add promise jobs, which is presumably why we see it in production.
Reproduction
npm i @electric-sql/client@1.5.28, then run the script below with Node. Output:
abort after 0 promise jobs: Cannot read properties of undefined (reading 'signal') (live requests: 1)
abort after 1 promise jobs: ok (live requests: 2)
...
The delay that triggers it shifts with the header function, so sweep abortPromiseJobs if a different setup shows no failure.
repro.mjs
import { ShapeStream } from '@electric-sql/client'
const upToDate = () =>
new Response(JSON.stringify([{ headers: { control: 'up-to-date' } }]), {
headers: {
'content-type': 'application/json',
'electric-cursor': '1',
'electric-handle': 'handle',
'electric-offset': '0_0',
'electric-schema': '{"id":{"type":"text"}}',
'electric-up-to-date': '',
},
})
const run = async abortPromiseJobs => {
const listeners = new Set()
const errors = []
let liveRequests = 0
const controller = new AbortController()
const stream = new ShapeStream({
url: 'http://localhost:3000/v1/shape',
params: { table: 'items' },
signal: controller.signal,
headers: { Authorization: async () => 'Bearer token' },
runtimeVisibility: {
getCurrentState: () => 'visible',
subscribe: listener => {
listeners.add(listener)
return () => listeners.delete(listener)
},
},
onError: error => {
errors.push(error)
},
fetchClient: async (input, init) => {
if (!new URL(String(input)).searchParams.has('live')) return upToDate()
liveRequests += 1
// A held long poll whose abort unwinds across a few promise jobs, as it
// does through a fetch wrapper.
return new Promise((_resolve, reject) => {
init.signal.addEventListener('abort', async () => {
for (let job = 0; job < abortPromiseJobs; job += 1) await null
reject(new DOMException('Aborted', 'AbortError'))
})
})
},
})
stream.subscribe(() => {})
await new Promise(resolve => setTimeout(resolve, 20))
// Pause and resume in one task.
for (const listener of listeners) listener('hidden')
for (const listener of listeners) listener('visible')
await new Promise(resolve => setTimeout(resolve, 20))
console.log(`abort after ${abortPromiseJobs} promise jobs: ${errors[0]?.message ?? 'ok'} (live requests: ${liveRequests})`)
controller.abort()
}
for (let jobs = 0; jobs <= 8; jobs += 1) await run(jobs)Suggested fix
Let a request clear only the controller it owns:
const requestAbortController = await this.#createAbortListener(signal)
// ...
finally {
if (this.#requestAbortController === requestAbortController) {
this.#requestAbortController = undefined
}
}That stops the crash, but the older #start() still runs #teardown() when it unwinds, after the newer run has begun. Having the resume wait for the previous run to settle would close that overlap too.
Workaround
Release the pause in a later task than the one that acquired it. Our adapter now defers visible with setTimeout(..., 0), which removed the error in every timing we tried.
Activity