Skip to content

Commit f6171ae

Browse files
authored
feat: diagnose selected PC discovery resolution (#151)
1 parent 3b1cf8e commit f6171ae

4 files changed

Lines changed: 89 additions & 8 deletions

File tree

docs/ble-connection-diagnostics.md

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,21 @@ An updated native/development app build from this branch is needed; the installe
99
| `ble_probe_connect` | Temporary GATT connection used during discovery |
1010
| `ble_probe_services` | Service discovery for a status probe |
1111
| `ble_status_read` | Discovery status-characteristic read |
12+
| `ble_status_parse` | Decode and parse the status; empty or invalid status fails |
13+
| `ble_selected_match` | Compare parsed status with the selected PC; `succeeded` or `not_matched` |
14+
| `ble_resolution` | Find and prepare the selected connection; `started`, `succeeded`, `failed`, or `timed_out` |
1215
| `ble_connect` | Connection to the selected device |
1316
| `ble_priority` | Optional Android priority request; failure is nonfatal |
1417
| `ble_mtu` | Android MTU negotiation |
1518
| `ble_services` | Service discovery for the control connection |
1619
| `ble_notifications` | Local notification listener registration, or a listener error |
1720
| `ble_notification_ready` | Android CCCD read and enabled-value check |
1821

19-
Each stage has `_started`, `_succeeded` and `_failed` outcomes. Listener registration success does not prove that the peripheral enabled notifications; Android checks its descriptor separately. Status-read success records a completed GATT read, not successful parsing. Scan probes can interleave; no device identifiers are included. Stop discovery and make one selected-device attempt for diagnosis.
22+
Unless noted above, stages have `_started`, `_succeeded` and `_failed` outcomes. Listener registration success does not prove that the peripheral enabled notifications; Android checks its descriptor separately. Status-read success records a completed GATT read, not successful parsing. Scan probes can interleave; no device identifiers are included. Stop discovery and make one selected-device attempt for diagnosis.
23+
24+
Resolution spans both discovery and connection preparation, including priority, MTU and service setup. Its timeout is the overall deadline, not necessarily a missing device. A successful selected match means identity equality, not authentication or completed handoff. Nonmatching candidates are informational and normal when other PCs are nearby. A parse failure after a successful read distinguishes invalid/empty status from a native read failure, but does not expose the rejected value or parsing reason. Cancellation may leave only `ble_resolution_started`; stale operations do not report terminal outcomes into newer attempts.
25+
26+
The S26 beta.22 test showed successful reads but no priority/MTU/notification stages for the selected probe, while the Linux probe recorded no writes or notification channel. Its log had no explicit resolution outcome, so timeout and matching/parsing causes were not distinguishable. These new events require an updated app build; the test does not establish a root cause or completed Linux support.
2027

2128
Cancelled operations cannot later append success/failure into a newer operation. Unsubscribed notification callbacks cannot append stage diagnostics. Cancellation may leave a started entry without an outcome; that is not proof of Bluetooth failure. Native rejection and timeout both count as failure of the relevant stage. Raw errors/codes, addresses, PC names, descriptor values, payloads and credentials are never added to stage diagnostics.
2229

src/diagnostics/DiagnosticLog.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,14 @@ const connectionStages = {
99
probe_connect: 'Connect for discovery status',
1010
probe_services: 'Discover status services',
1111
status_read: 'Read discovery status',
12+
status_parse: 'Parse discovery status',
13+
selected_match: 'Match discovery status to the selected PC',
14+
resolution: 'Resolve and prepare the selected PC connection',
1215
notifications: 'Register notification listener',
1316
notification_ready: 'Verify Android notification descriptor',
1417
} as const;
1518
export type ConnectionStage = keyof typeof connectionStages;
16-
export type ConnectionStageOutcome = 'started' | 'succeeded' | 'failed';
19+
export type ConnectionStageOutcome = 'started' | 'succeeded' | 'failed' | 'not_matched' | 'timed_out';
1720

1821
const messages = {
1922
scan_started: 'Looking for nearby PCs.',
@@ -46,7 +49,7 @@ export class DiagnosticLog {
4649
}
4750
addConnectionStage(stage: ConnectionStage, outcome: ConnectionStageOutcome): void {
4851
// Only fixed vocabulary crosses this boundary: no native error, address or payload.
49-
this.#append(`ble_${stage}_${outcome}`, `${connectionStages[stage]}: ${outcome}.`, outcome === 'failed' ? 'warning' : 'info');
52+
this.#append(`ble_${stage}_${outcome}`, `${connectionStages[stage]}: ${outcome}.`, outcome === 'failed' || outcome === 'timed_out' ? 'warning' : 'info');
5053
}
5154
#append(code: string, message: string, level: DiagnosticLevel): void {
5255
this.#entries = [{ id: this.#nextId++, timestamp: Date.now(), level, code, message }, ...this.#entries].slice(0, 200);

src/transport/ReactNativeBleTransport.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,65 @@ function manager(overrides: Record<string, unknown> = {}): BleManager {
3030
}
3131

3232
describe('ReactNativeBleTransport', () => {
33+
it.each(['', 'private malformed', '{"protocolVersion":2}', '{"protocolVersion":1,"desktopId":"other-private"}'])('diagnoses rejected or nonmatching status (%s)', async (raw) => {
34+
const log = new DiagnosticLog();
35+
let callback!: (error: Error | null, value: Device | null) => void;
36+
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), 'ios', 100, undefined, log);
37+
const result = transport.resolveAndConnect('pc-1');
38+
const rejected = expect(result).rejects.toThrow('timed out');
39+
await waitFor(() => !!callback);
40+
callback(null, device({ readCharacteristicForService: jest.fn(async () => ({ value: fromByteArray(new TextEncoder().encode(raw)) } as Characteristic)) }));
41+
await rejected;
42+
const codes = log.snapshot().map((entry) => entry.code);
43+
expect(codes).toContain(raw.includes('other-private') ? 'ble_selected_match_not_matched' : 'ble_status_parse_failed');
44+
expect(codes.filter((code) => code === 'ble_resolution_timed_out')).toHaveLength(1);
45+
expect(codes).not.toContain('ble_resolution_succeeded');
46+
expect(log.export()).not.toMatch(/private|pc-1/);
47+
const before = log.export();
48+
callback(null, device());
49+
await Promise.resolve();
50+
expect(log.export()).toBe(before);
51+
await transport.disconnect();
52+
});
53+
54+
it('records a matching handoff even with throwing diagnostic observers', async () => {
55+
const log = new DiagnosticLog();
56+
log.subscribe(() => { throw new Error('observer'); });
57+
let callback!: (error: Error | null, value: Device | null) => void;
58+
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), 'android', 1000, undefined, log);
59+
const result = transport.resolveAndConnect('pc-1');
60+
await waitFor(() => !!callback);
61+
callback(null, device());
62+
await result;
63+
const codes = log.snapshot().map((entry) => entry.code);
64+
expect(codes).toContain('ble_status_parse_succeeded');
65+
expect(codes).toContain('ble_selected_match_succeeded');
66+
expect(codes[0]).toBe('ble_resolution_succeeded');
67+
await transport.disconnect();
68+
});
69+
70+
it('does not report cancelled resolution as a timeout or failure', async () => {
71+
const log = new DiagnosticLog();
72+
let callback!: (error: Error | null, value: Device | null) => void;
73+
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn((_u, _o, cb) => { callback = cb; }) }), 'ios', 1000, undefined, log);
74+
const result = transport.resolveAndConnect('pc-1');
75+
const rejected = expect(result).rejects.toThrow('cancelled');
76+
await waitFor(() => !!callback);
77+
await transport.disconnect();
78+
await rejected;
79+
callback(null, device());
80+
await Promise.resolve();
81+
expect(log.snapshot().map((entry) => entry.code)).toEqual(['ble_resolution_started']);
82+
});
83+
84+
it('records scan startup failure without native error details', async () => {
85+
const log = new DiagnosticLog();
86+
const transport = new ReactNativeBleTransport(manager({ startDeviceScan: jest.fn(() => { throw new Error('private'); }) }), 'ios', 100, undefined, log);
87+
await expect(transport.resolveAndConnect('pc-1')).rejects.toThrow('discovery failed');
88+
expect(log.snapshot().map((entry) => entry.code)).toEqual(['ble_resolution_failed', 'ble_resolution_started']);
89+
expect(log.export()).not.toContain('private');
90+
await transport.disconnect();
91+
});
3392
it('records discovery status separately from the selected-PC connection', async () => {
3493
const log = new DiagnosticLog();
3594
let scanCallback!: (error: Error | null, value: Device | null) => void;
@@ -43,6 +102,7 @@ describe('ReactNativeBleTransport', () => {
43102
'ble_probe_connect_started', 'ble_probe_connect_succeeded',
44103
'ble_probe_services_started', 'ble_probe_services_succeeded',
45104
'ble_status_read_started', 'ble_status_read_succeeded',
105+
'ble_status_parse_started', 'ble_status_parse_succeeded',
46106
]);
47107
stop();
48108
});

src/transport/ReactNativeBleTransport.ts

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -92,21 +92,24 @@ export class ReactNativeBleTransport implements BleTransport {
9292
async resolveAndConnect(desktopId: string): Promise<DiscoveredDesktop> {
9393
await this.disconnect();
9494
const operation = ++this.#operation;
95+
this.#recordStage('resolution', 'started', operation);
9596
return new Promise<DiscoveredDesktop>((resolve, reject) => {
9697
let active = true;
9798
let claimedDeviceId: string | null = null;
9899
let cancellation: Promise<void> | null = null;
99100
const succeed = (desktop: DiscoveredDesktop) => {
100101
if (!active) return;
102+
this.#recordStage('resolution', 'succeeded', operation);
101103
active = false;
102104
clearTimeout(timer);
103105
this.#managerOrCreate().stopDeviceScan();
104106
this.#scanKeys.clear();
105107
if (this.#resolutionCancel === cancel) this.#resolutionCancel = null;
106108
resolve(desktop);
107109
};
108-
const cancel = (error: Error): Promise<void> => {
110+
const cancel = (error: Error, outcome: 'failed' | 'timed_out' = 'failed'): Promise<void> => {
109111
if (!active) return cancellation ?? Promise.resolve();
112+
this.#recordStage('resolution', outcome, operation);
110113
active = false;
111114
clearTimeout(timer);
112115
this.#managerOrCreate().stopDeviceScan();
@@ -127,14 +130,16 @@ export class ReactNativeBleTransport implements BleTransport {
127130
return cancellation;
128131
};
129132
this.#resolutionCancel = cancel;
130-
const timer = setTimeout(() => { void cancel(new Error('Saved PC discovery timed out.')); }, this.nativeTimeoutMs);
133+
const timer = setTimeout(() => { void cancel(new Error('Saved PC discovery timed out.'), 'timed_out'); }, this.nativeTimeoutMs);
131134
const onAdvertisement = (error: Error | null, device: Device | null) => {
132135
if (!active || operation !== this.#operation) return;
133136
if (error) { void cancel(new Error('Saved PC discovery failed.')); return; }
134137
if (!device || this.#scanDevices.has(device.id) || this.#scanKeys.has(this.#scanKey(device)) || this.#scanDevices.size >= 4) return;
135138
this.#scanDevices.set(device.id, device);
136139
this.#scanKeys.add(this.#scanKey(device));
137140
const task = this.#readStatus(device, (desktop) => {
141+
if (!active || operation !== this.#operation) return false;
142+
this.#recordStage('selected_match', desktop.desktopId === desktopId ? 'succeeded' : 'not_matched', operation);
138143
if (desktop.desktopId !== desktopId || claimedDeviceId !== null) return false;
139144
claimedDeviceId = device.id;
140145
return true;
@@ -347,9 +352,15 @@ export class ReactNativeBleTransport implements BleTransport {
347352
}
348353
await this.#stage('probe_services', () => this.#bounded(target.discoverAllServicesAndCharacteristics()), operation);
349354
const characteristic = await this.#stage('status_read', () => this.#bounded(target.readCharacteristicForService(BLE_UUIDS.service, BLE_UUIDS.status)), operation);
350-
if (!characteristic.value) return null;
351-
const raw = new TextDecoder().decode(toByteArray(characteristic.value));
352-
const status = parseStatus(raw);
355+
this.#recordStage('status_parse', 'started', operation);
356+
let status;
357+
try {
358+
status = characteristic.value ? parseStatus(new TextDecoder().decode(toByteArray(characteristic.value))) : null;
359+
} catch (error) {
360+
this.#recordStage('status_parse', 'failed', operation);
361+
throw error;
362+
}
363+
this.#recordStage('status_parse', status ? 'succeeded' : 'failed', operation);
353364
const desktop = status ? {
354365
...status,
355366
displayName: desktopDisplayName(status, { name: device.name, localName: device.localName }, this.platform),

0 commit comments

Comments
 (0)