Skip to content

Bound the BLE connect and file-read waits so they report instead of hanging #546

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.

Sign up for GitHub

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

Merged
makermelissa merged 10 commits into from
Aug 21, 2026
+212 −46
Merged

Bound the BLE connect and file-read waits so they report instead of hanging #546

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cb1a134
Bound the BLE connect so it can't hang forever
dhalbert Aug 8, 2026
c84e3a7
Report a failed BLE file read instead of hanging on it
dhalbert Aug 8, 2026
f6ac81b
Shorten the advertisement wait and say something during it
dhalbert Aug 8, 2026
0c197d4
Bound the silent reconnect's connect too
dhalbert Aug 8, 2026
f7edefe
Abort the advertisement watch before connecting, not after
dhalbert Aug 9, 2026
3113ab6
Correct the ble.js comments to match what the investigation established
dhalbert Aug 11, 2026
2bd1935
Don't name a device in the status while racing several
dhalbert Aug 18, 2026
8c66922
Merge remote-tracking branch 'circuitpython/main' into bound-ble-conn…
dhalbert Aug 19, 2026
e6d68ce
Take ble-file-transfer-js 1.1.0 and drop the local read guard
dhalbert Aug 19, 2026
4a89ca5
Force a CCCD write when subscribing to BLE serial
dhalbert Aug 20, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions index.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ <h1>Select USB Host Folder</h1>
</tr>
</tbody>
</table>
<div id="message"></div>
<div id="firmware-update" class="firmware-update-suggestion-container"></div>
<h3>More network devices<i class="refresh fa-solid fa-sync-alt" title="Refresh Device List"></i></h3>
<div id="devices"></div>
Expand Down Expand Up @@ -435,6 +436,7 @@ <h3>More network devices<i class="refresh fa-solid fa-sync-alt" title="Refresh D
</tr>
</tbody>
</table>
<div id="message"></div>
<div id="firmware-update" class="firmware-update-suggestion-container"></div>
<div class="buttons centered">
<button class="purple-button ok-button">Close</button>
Expand Down
29 changes: 27 additions & 2 deletions js/common/dialogs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -340,9 +340,28 @@ class ButtonValueDialog extends GenericModal {
}
}

// Report a failed device-info read in the dialog. Without this the read's
// rejection escapes as an unhandled promise rejection and the dialog is simply
// left blank, which reads as "the device answered with nothing" rather than
// "we never reached the device".
function showDeviceInfoError(modal, error) {
console.error("Unable to read device info:", error);
const msgElement = modal.querySelector("#message");
if (msgElement) {
msgElement.textContent =
"Could not read device information. The connection to the device was lost.";
}
}

class DiscoveryModal extends GenericModal {
async _getVersionInfo() {
const deviceInfo = await this._showBusy(this._fileHelper.versionInfo());
let deviceInfo;
try {
deviceInfo = await this._showBusy(this._fileHelper.versionInfo());
} catch (error) {
showDeviceInfoError(this._currentModal, error);
return;
}
this._currentModal.querySelector("#version").textContent = deviceInfo.version;
const boardLink = this._currentModal.querySelector("#board");
boardLink.href = `https://circuitpython.org/board/${deviceInfo.board_id}/`;
Expand Down Expand Up @@ -413,7 +432,13 @@ class DiscoveryModal extends GenericModal {

class DeviceInfoModal extends GenericModal {
async _getDeviceInfo() {
const deviceInfo = await this._showBusy(this._fileHelper.versionInfo());
let deviceInfo;
try {
deviceInfo = await this._showBusy(this._fileHelper.versionInfo());
} catch (error) {
showDeviceInfoError(this._currentModal, error);
return;
}
this._currentModal.querySelector("#version").textContent = deviceInfo.version;
const boardLink = this._currentModal.querySelector("#board");
boardLink.href = `https://circuitpython.org/board/${deviceInfo.board_id}/`;
Expand Down
217 changes: 179 additions & 38 deletions js/workflows/ble.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,31 @@ const POST_OP_DISCONNECT_GRACE_MS = 4000;
// Wait after GATT reconnects so the VM finishes booting before the next op.
const POST_RECONNECT_SETTLE_MS = 2000;

// How long to wait for an advertisement before connecting anyway. Chrome's
// BlueZ backend never delivers advertisementreceived, so on Linux this event
// does not arrive at all and an unbounded wait leaves the connect dialog open
// forever with no feedback. macOS delivers the first event within ~30ms, so a
// couple of seconds is generous everywhere it works. On Linux the wait is not
// wasted even though nothing arrives: the discovery session that
// watchAdvertisements() opens is what makes BlueZ (re)create its device object,
// without which gatt.connect() rejects as "no longer in range".
const ADVERTISEMENT_WAIT_MS = 2000;
// How long to allow gatt.connect() before giving up. Chrome bounds this itself
// at ~41s on Linux, but not while a watchAdvertisements() watch is armed -- in
// that state the promise simply never settles, which is the case this timeout
// exists for. The ceiling is deliberately loose rather than tuned: a healthy
// adapter connects in well under a second (0.5s on macOS and Windows, 0.7s
// median on an Intel AX210 on Linux), but a user's host controller may be far
// slower -- 26.6s was measured on a faulty MediaTek MT7920. The point is to
// convert a never-settling promise into a reportable failure, not to enforce a
// tight deadline.
const CONNECT_TIMEOUT_MS = 30000;
// Per-attempt bound for the silent reconnect after a firmware autoreload.
// Shorter than CONNECT_TIMEOUT_MS because this path runs once per entry in
// RECONNECT_DELAYS_MS, and a reconnect that has not landed within ten seconds
// has stopped being silent regardless of whether it eventually succeeds.
const SILENT_RECONNECT_TIMEOUT_MS = 10000;

let btnRequestBluetoothDevice, btnReconnect;

class BLEWorkflow extends Workflow {
Expand Down Expand Up @@ -62,6 +87,11 @@ class BLEWorkflow extends Workflow {
// Track in-flight watchAdvertisements abort controllers so we can
// cancel them when any device wins or when we tear down (#410).
this._pendingAdvAborts = new Set();

// Only one device may attempt a connection at a time. Without this,
// several remembered devices whose advertisement waits expire together
// would all try to connect at once.
this._connectAttemptInFlight = false;
}

// Called by the FileTransferClient wrapper right before any mutating
Expand Down Expand Up @@ -131,10 +161,7 @@ class BLEWorkflow extends Workflow {
}
// Cancel any in-flight watchAdvertisements so a subsequent reconnect
// doesn't pile up Chrome's per-device watch quota (#410).
for (const ctrl of this._pendingAdvAborts) {
ctrl.abort();
}
this._pendingAdvAborts.clear();
this._abortAdvWatches();
await super.onDisconnected(e, reconnect);
}

Expand Down Expand Up @@ -197,6 +224,21 @@ class BLEWorkflow extends Workflow {
// Use cached bound handler so removeEventListener actually matches.
this.txCharacteristic.removeEventListener('characteristicvaluechanged', this._onSerialReceiveBound);
this.txCharacteristic.addEventListener('characteristicvaluechanged', this._onSerialReceiveBound);

// Stop before starting, so a CCCD write actually goes out. Reconnecting to a
// bonded board, startNotifications() can return without writing the
// descriptor, leaving the board with notifications disabled -- the terminal
// then stays silent for the rest of the session while file transfer works.
// Measured on a Feather nRF52840 and a Metro ESP32-S3, on Linux and Windows.
//
// No read is needed first here, unlike the file transfer client's own
// subscribe: switchToDevice() bonds through the file transfer client before
// calling this, so the link is already encrypted by now.
try {
await this.txCharacteristic.stopNotifications();
} catch (e) {
// Nothing was subscribed yet, which is the ordinary first connect.
}
await this.txCharacteristic.startNotifications();
return true;
} catch (e) {
Expand All @@ -213,6 +255,7 @@ class BLEWorkflow extends Workflow {
const devices = await navigator.bluetooth.getDevices();

console.log('> Found ' + devices.length + ' Bluetooth device(s).');
this._showSearchingStatus(devices);
// These devices may not be powered on or in range, so scan for
// advertisement packets from them before connecting.
for (const device of devices) {
Expand All @@ -234,72 +277,165 @@ class BLEWorkflow extends Workflow {
});
}

// Say something during the advertisement wait. On Linux it always runs to the
// full timeout, and silence looks like a hang. Naming a device is only honest
// when there is one: the reconnect paths race every permitted device and
// connect to whichever answers first, which need not be the one named.
_showSearchingStatus(devices) {
if (devices.length === 0) {
return;
}
this.clearConnectStatus();
this.showConnectStatus(devices.length === 1
? "Looking for " + devices[0].name + "..."
: "Looking for " + devices.length + " previously connected boards...");
}

// Abort pending advertisement watches, optionally sparing one. Deleting
// while iterating a Set is safe.
_abortAdvWatches(keep = null) {
for (const ctrl of this._pendingAdvAborts) {
if (ctrl !== keep) {
ctrl.abort();
this._pendingAdvAborts.delete(ctrl);
}
}
}

async connectToBluetoothDevice(device) {
const abortController = new AbortController();
this._pendingAdvAborts.add(abortController);
let advHandled = false;

async function onAdvertisementReceived(event) {
// Multiple ads can land in the same event-loop tick before
// abortController.abort() takes effect on the listener. Guard
// so we only run the connect flow once per device. See #410.
if (advHandled) {
// Runs either when an advertisement arrives or when we give up waiting
// for one. Guarded because multiple ads can land in the same event-loop
// tick before abortController.abort() takes effect on the listener, and
// because the timer can fire alongside a late advertisement. See #410.
const attemptConnect = async (reason) => {
if (advHandled || this._connectAttemptInFlight) {
return;
}
advHandled = true;
console.log('> Received advertisement from "' + device.name + '"...');
// This device won. Abort ALL pending watchAdvertisements
// (including this one) so other paired devices stop scanning
// and don't pile up Chrome's per-device watch quota.
for (const ctrl of this._pendingAdvAborts) {
ctrl.abort();
}
this._pendingAdvAborts.clear();
console.log('Connecting to GATT Server from "' + device.name + '"...');
this._connectAttemptInFlight = true;
clearTimeout(advTimer);

// This device won, so stop every pending watch, this device's
// included. The reason is the one from #410: Chrome enforces a
// per-device watchAdvertisements quota, and leaving the losers
// armed piles up against it.
//
// An earlier version of this comment claimed the abort was needed
// because connecting while a BlueZ discovery session is active
// fails on Linux. That was investigated at length and does not
// hold: the connect failures it described were the host Bluetooth
// controller (a MediaTek MT7920, 0/40 while WiFi scanned), not the
// discovery state, and the same board connects 20/20 on an Intel
// AX210 with a watch armed or not. The kernel also disables
// scanning ~1.5ms before every create-connection regardless of
// what BlueZ believes, so aborting the watch does not change the
// controller's state at the moment of connect.
//
// Ordering it before the connect is therefore housekeeping, not a
// workaround, and on Linux it may even cost a little: Chrome's
// discovery session is what refreshes BlueZ's 30s sighting window,
// and gatt.connect() rejects with "no longer in range" once that
// window lapses.
this._abortAdvWatches();
try {
this.bleServer = await device.gatt.connect();
} catch (error) {
console.log(error);
// TODO(ericzundel): Add to suggestBLEConnectAction if we can determine the exception type
this.showConnectStatus("Failed to connect to device. Try forgetting device from OS bluetooth devices and try again.");
// Disable the reconnect button
this.connectionStep(1);
}
if (this.bleServer && this.bleServer.connected) {
console.log('> Bluetooth device "' + device.name + ' connected.');
await this.switchToDevice(device);
} else {
console.log('Unable to connect to bluetooth device "' + device.name + '.');
await this._connectToGattServer(device, reason);
} finally {
this._connectAttemptInFlight = false;
}
}
};

const advTimer = setTimeout(
() => attemptConnect(`no advertisement within ${ADVERTISEMENT_WAIT_MS / 1000}s`),
ADVERTISEMENT_WAIT_MS);

// Use the abortController signal so we don't need to manage the
// handler reference manually — the listener is auto-removed when
// onAdvertisementReceived calls abortController.abort().
device.addEventListener('advertisementreceived',
onAdvertisementReceived.bind(this),
{signal: abortController.signal});
// abortController.abort() is called.
device.addEventListener('advertisementreceived', () => {
console.log('> Received advertisement from "' + device.name + '"...');
attemptConnect('advertisement received');
}, {signal: abortController.signal});

this.debugLog("Attempting to connect to " + device.name + "...");
try {
this.clearConnectStatus();
// No status message here. The caller has already said what it is
// looking for, and naming this device would be wrong: the reconnect
// paths arm a watch on every permitted device at once, so each call
// would overwrite the last and leave a loser's name on screen.
console.log('Watching advertisements from "' + device.name + '"...');
console.log('If no advertisements are received, make sure the device is powered on and in range. You can also try resetting the device.');
await device.watchAdvertisements({signal: abortController.signal});
}
catch (error) {
clearTimeout(advTimer);
console.error(error);
this.showConnectStatus(this._suggestBLEConnectActions(error));
}
}

// Connect with a bound. gatt.connect() does not always reject on its own --
// on Linux with a watch armed it never settles -- so race it against a timer
// and cancel with gatt.disconnect(), which is the only way page JS can abort
// an in-flight connect. Chrome has honoured disconnect() as a cancel since
// M140; before that the attempt is orphaned rather than aborted, so treat a
// timeout as fatal rather than assuming the adapter is left clean.
async _connectWithTimeout(device, timeoutMs) {
let connectTimer;
try {
return await Promise.race([

@makermelissa makermelissa Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could use the timeout function in https://github.com/circuitpython/web-editor/blob/main/js/common/utilities.js#L2 here, which basically already does this.

@dhalbert dhalbert Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude wrote this:

Switched to timeout():

async _connectWithTimeout(device, timeoutMs) {
    try {
        return await timeout(() => device.gatt.connect(), timeoutMs);
    } catch (error) {
        device.gatt.disconnect();
        throw error;
    }
}

device.gatt.disconnect() has to stay. timeout() abandons the promise, and an abandoned gatt.connect() keeps running in the browser process; disconnect() is the only way page JS can cancel it, and Chrome has honoured it as a cancel since M140. Calling it on any rejection rather than only on the timeout is what keeps this to a try/catch — disconnect() on a device that is not connected is a no-op.

Also needs timeout added to the utilities.js import. One small loss: a timeout now logs Timed Out rather than connect did not complete within 30s, which was console-only.

@dhalbert dhalbert Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does the above look to you? I can submit a commit for that?

@makermelissa makermelissa Aug 19, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we update timeout to work better? One of the reasons it's nice to have in one place is updating it improves it everywhere.

@dhalbert dhalbert Aug 19, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Claude wrote this:

Yes. Two things, for every caller: clear the timer when the callback wins, and pass the callback an AbortSignal that fires on timeout so it can stop what it started — Promise.race() abandons the loser, it doesn't cancel it. Today that leaves the BLE connect running in the browser process, and the connect wait in web.js polling every 100 ms for the life of the page.

// Run the callback and if it doesn't complete in the given time, throw an error.
// The callback is given an AbortSignal that fires on timeout: a raced promise is
// abandoned, not stopped, so anything it started has to be cancelled explicitly.
async function timeout(callback, ms) {
    const controller = new AbortController();
    let timer;
    try {
        return await Promise.race([
            callback(controller.signal),
            new Promise((_, reject) => {
                timer = setTimeout(() => {
                    controller.abort();
                    reject(new Error("Timed Out"));
                }, ms);
            }),
        ]);
    } finally {
        clearTimeout(timer);
    }
}

Existing callers ignore the extra argument. The BLE side becomes:

async _connectWithTimeout(device, timeoutMs) {
    return await timeout((signal) => {
        signal.addEventListener("abort", () => device.gatt.disconnect(), {once: true});
        return device.gatt.connect();
    }, timeoutMs);
}

Want the web.js loop fixed here too (&& !signal.aborted), or separately?

@makermelissa makermelissa Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can do it in a separate pr.

device.gatt.connect(),
new Promise((_, reject) => {
connectTimer = setTimeout(() => {
device.gatt.disconnect();
reject(new Error(
`connect did not complete within ${timeoutMs / 1000}s`));
}, timeoutMs);
}),
]);
} finally {
clearTimeout(connectTimer);
}
}

async _connectToGattServer(device, reason) {
console.log(`Connecting to GATT Server from "${device.name}" (${reason})...`);
this.showConnectStatus("Connecting to " + device.name + "...");

try {
this.bleServer = await this._connectWithTimeout(device, CONNECT_TIMEOUT_MS);
} catch (error) {
console.log(error);
// TODO(ericzundel): Add to suggestBLEConnectAction if we can determine the exception type
this.showConnectStatus(
`Could not connect to ${device.name}. Try again. If it keeps failing, forget the ` +
`device in your operating system's Bluetooth settings, then reload this page.`);
// Disable the reconnect button
this.connectionStep(1);
return;
}

if (this.bleServer && this.bleServer.connected) {
console.log('> Bluetooth device "' + device.name + '" connected.');
await this.switchToDevice(device);
} else {
console.log('Unable to connect to bluetooth device "' + device.name + '".');
this.showConnectStatus(`Could not connect to ${device.name}. Try again.`);
this.connectionStep(1);
}
}

// Request Bluetooth Device
async onRequestBluetoothDeviceButtonClick(e) {
console.log('Requesting any Bluetooth device...');
this.debugLog("Requesting device. Cancel if empty and try existing");
let device = await this.requestDevice();

console.log('> Requested ' + device.name);
this._showSearchingStatus([device]);
await this.connectToBluetoothDevice(device);
}

Expand Down Expand Up @@ -382,6 +518,7 @@ class BLEWorkflow extends Workflow {
if (!this.bleDevice) {
try {
let devices = await navigator.bluetooth.getDevices();
this._showSearchingStatus(devices);
for (const device of devices) {
await this.connectToBluetoothDevice(device);
}
Expand All @@ -405,7 +542,11 @@ class BLEWorkflow extends Workflow {
await sleep(delay);
try {
console.log(`Silent reconnect: attempting after ${delay}ms…`);
this.bleServer = await this.bleDevice.gatt.connect();
// Bounded: an unbounded connect here stalls the whole
// reconnect ladder, and every mutating op waits on it via
// awaitPostOpReconnect(), so a save appears to hang.
this.bleServer = await this._connectWithTimeout(
this.bleDevice, SILENT_RECONNECT_TIMEOUT_MS);
if (this.bleServer && this.bleServer.connected) {
console.log('Silent reconnect: GATT reconnected, rebinding characteristics…');
await this._rebindAfterSilentReconnect();
Expand Down
8 changes: 3 additions & 5 deletions package-lock.json
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading