Skip to content

Commit 951ce8d

Browse files
refactor: address technical audit and finalize Bluetooth retry logic
- Restored fast-retry loop for initial GATT connections to avoid UX degradation. - Fixed misleading return value in public connect function. - Enhanced race condition prevention in reconnection flow. - Simplified JSDoc in reconnection constants. - Updated unit tests. Co-authored-by: arii <342438+arii@users.noreply.github.com>
1 parent e292721 commit 951ce8d

2 files changed

Lines changed: 42 additions & 30 deletions

File tree

constants/bluetooth-reconnection.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,6 @@
44
*/
55

66
// Maximum number of times to attempt reconnection before giving up.
7-
// Note: We avoid importing @/lib/env here to prevent client-side crashes,
8-
// as that module performs server-side environment validation.
9-
// We use process.env directly which is handled by Next.js at build time.
107
export const BLUETOOTH_MAX_RECONNECT_ATTEMPTS =
118
typeof process !== 'undefined' &&
129
process.env.NEXT_PUBLIC_BLUETOOTH_MAX_RECONNECT_ATTEMPTS
@@ -18,14 +15,15 @@ export const RECONNECT_BASE_DELAY_MS = 2000
1815
export const RECONNECT_DELAY_INCREMENT_MS = 500
1916
export const RECONNECT_RANDOM_DELAY_MS = 1000
2017

18+
// Fast Retry Parameters for initial connection attempts
19+
export const FAST_RECONNECT_DELAY_MS = 1000
20+
export const FAST_RECONNECT_MAX_ATTEMPTS = 3
21+
2122
/**
22-
* Calculates the backoff delay for a given reconnection attempt.
23-
* Uses a linear backoff strategy with jitter.
24-
* @param attempt The current attempt number (starting from 1).
25-
* @returns The delay in milliseconds.
23+
* @param attempt - The current attempt number (starting from 1).
24+
* @returns The delay in milliseconds using a linear backoff strategy with jitter.
2625
*/
2726
export const getBackoffDelay = (attempt: number): number => {
28-
// Linear backoff: base + (attempt-1) * increment + jitter
2927
const increment = (attempt - 1) * RECONNECT_DELAY_INCREMENT_MS
3028
const jitter = Math.random() * RECONNECT_RANDOM_DELAY_MS
3129
return RECONNECT_BASE_DELAY_MS + increment + jitter

hooks/useBluetoothHRM.ts

Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,8 @@ import { BLUETOOTH_MESSAGES } from '@/constants/bluetooth-messages'
1414
import {
1515
BLUETOOTH_MAX_RECONNECT_ATTEMPTS,
1616
getBackoffDelay,
17+
FAST_RECONNECT_DELAY_MS,
18+
FAST_RECONNECT_MAX_ATTEMPTS,
1719
} from '@/constants/bluetooth-reconnection'
1820

1921
const HR_SERVICE_UUID = 'heart_rate'
@@ -325,6 +327,7 @@ const useBluetoothHRM = (props: UseBluetoothHRMProps = {}) => {
325327
reconnectTimeoutRef.current = setTimeout(() => {
326328
if (
327329
statusRef.current !== BluetoothConnectionStatus.CONNECTED &&
330+
!isConnecting.current &&
328331
!isManualDisconnect.current
329332
) {
330333
connectToGattRef.current?.(device, true).catch((error: unknown) => {
@@ -446,14 +449,39 @@ const useBluetoothHRM = (props: UseBluetoothHRMProps = {}) => {
446449
)
447450
}
448451

449-
const server = await cancellablePromise(device.gatt!.connect(), {
450-
timeoutMs: 20000,
451-
errorMessage: 'GATT connection timeout',
452-
signal: abortControllerRef.current.signal,
453-
})
452+
let server: BluetoothRemoteGATTServer | undefined
453+
for (
454+
let attempt = 1;
455+
attempt <= FAST_RECONNECT_MAX_ATTEMPTS;
456+
attempt++
457+
) {
458+
try {
459+
server = await cancellablePromise(device.gatt!.connect(), {
460+
timeoutMs: 20000,
461+
errorMessage: 'GATT connection timeout',
462+
signal: abortControllerRef.current.signal,
463+
})
464+
break
465+
} catch (error) {
466+
const isBusy =
467+
String(error).includes('busy') ||
468+
String(error).includes('NetworkError')
469+
if (isBusy && attempt < FAST_RECONNECT_MAX_ATTEMPTS) {
470+
logger.warn(
471+
{ device: device.name, attempt },
472+
'GATT connection busy, fast-retrying...'
473+
)
474+
await new Promise((res) =>
475+
setTimeout(res, FAST_RECONNECT_DELAY_MS)
476+
)
477+
continue
478+
}
479+
throw error
480+
}
481+
}
454482

455483
if (abortControllerRef.current?.signal.aborted) {
456-
server.disconnect()
484+
server?.disconnect()
457485
throw new DOMException('Connection aborted', 'AbortError')
458486
}
459487

@@ -714,22 +742,8 @@ const useBluetoothHRM = (props: UseBluetoothHRMProps = {}) => {
714742

715743
if (device) {
716744
logger.info({ device: device.name }, 'Connecting to device')
717-
try {
718-
await connectToGatt(device)
719-
return true
720-
} catch (error) {
721-
if (silent) throw error
722-
723-
const isBusy =
724-
String(error).includes('busy') ||
725-
String(error).includes('NetworkError')
726-
if (isBusy) {
727-
reconnectAttempts.current = 0
728-
reconnect(device, String(error))
729-
return true
730-
}
731-
throw error
732-
}
745+
await connectToGatt(device)
746+
return true
733747
} else if (!silent) {
734748
logger.info('No device to connect')
735749
throw new Error('No device found or selected for connection.')

0 commit comments

Comments
 (0)