Skip to content

Commit 891de57

Browse files
Lykhoydaclaude
andauthored
fix(cdp): fast-fail cdp_status when the Dev Client picker blocks the bundle (closes #184) (#187)
cdp_status with the Expo Dev Client "Development servers" picker up AND Metro running hung ~33.5s: stale C++ targets pass filterValidTargets, the 1+1 probe passes, and performSetup's waitForReact then burns its full 30s before returning a misleading ok:true. Fix: thread a connect intent ('default' | 'status') through autoConnect -> discoverAndConnect -> connectToTarget. For a status-scoped connect against a non-Hermes target, run a bounded React-reachability probe (default 5s, RN_PICKER_PROBE_BUDGET_MS-overridable) BEFORE setup()'s 30s waitForReact. If React isn't reachable, throw the typed PickerBlockingBundleError, which bypasses both the connectToTarget retry loop and the discoverAndConnect candidate loop; status.ts maps it to a fast failResult with code PICKER_BLOCKING and an actionable message ("select your Metro server, then retry cdp_status"). Deliberately NOT changing the global REACT_READY_TIMEOUT_MS (shared by every connect path) and NOT adding a pre-autoConnect target-shape heuristic (which false-positives on healthy reloads). Hermes targets skip the probe entirely, so legitimate slow first-builds keep the full budget; non-status connect paths are unchanged. New pure helpers (tested without a real WebSocket, per the repo's formatConnectFailureMessage/stickyPlatformFilters convention): - probeReactReachable(evaluate, budgetMs, pollMs) -> boolean (setup.ts) - shouldRunPickerProbe(intent, target), PickerBlockingBundleError (connect.ts) Full suite 1563 pass; tsc clean. End-to-end picker-up assertion belongs to the deferred Dev Client harness CI (DC-Task 9 / #136). Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 056532b commit 891de57

10 files changed

Lines changed: 276 additions & 19 deletions

File tree

scripts/cdp-bridge/dist/cdp-client.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,11 @@ export class CDPClient {
132132
}
133133
return ok;
134134
}
135-
async autoConnect(portHint, filtersOrPlatform) {
135+
async autoConnect(portHint, filtersOrPlatform, intent = 'default') {
136136
const filters = typeof filtersOrPlatform === 'string'
137137
? { platform: filtersOrPlatform }
138138
: (filtersOrPlatform ?? {});
139-
return autoConnectFn(this.buildConnectCtx(), portHint, filters);
139+
return autoConnectFn(this.buildConnectCtx(), portHint, filters, intent);
140140
}
141141
async listTargets(portHint) {
142142
return discoverForList(this._port, portHint);

scripts/cdp-bridge/dist/cdp/connect.js

Lines changed: 66 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,41 @@ import { resolveBundleId } from '../project-config.js';
44
import { discover } from './discovery.js';
55
import { sleep } from './state.js';
66
import { CDP_TIMEOUT_FAST } from './timeout-config.js';
7-
export async function autoConnect(ctx, portHint, filters) {
7+
import { probeReactReachable } from './setup.js';
8+
// Budget for the status-scoped React-reachability probe. Generous enough to
9+
// clear a normal Bridgeless reload (React ready in <~2s) but far below setup()'s
10+
// 30s waitForReact, so cdp_status fails fast instead of hanging when the Dev
11+
// Client picker blocks the bundle. Env-overridable for tuning/tests.
12+
const PICKER_PROBE_BUDGET_MS = (() => {
13+
const n = parseInt(process.env.RN_PICKER_PROBE_BUDGET_MS ?? '', 10);
14+
return Number.isFinite(n) && n > 0 ? n : 5000;
15+
})();
16+
/**
17+
* GH #184: thrown when a status-scoped connect can't reach React within the
18+
* bounded budget on a non-Hermes target — the signature of the Expo Dev Client
19+
* "Development servers" picker leaving stale C++ targets advertised while the
20+
* bundle never loads. status.ts maps it to a fast, actionable failResult
21+
* instead of letting setup() burn the full 30s waitForReact.
22+
*/
23+
export class PickerBlockingBundleError extends Error {
24+
target;
25+
constructor(target) {
26+
super(`Dev Client picker appears to be blocking the bundle: React was not reachable on target ` +
27+
`"${target.title}" (vm=${target.vm}). If the Expo "Development servers" picker is showing on ` +
28+
`the simulator, select your Metro server, then retry cdp_status. (If the bundle is still building, just retry.)`);
29+
this.name = 'PickerBlockingBundleError';
30+
this.target = target;
31+
}
32+
}
33+
/**
34+
* GH #184: run the bounded picker probe only for a status-intent connect against
35+
* a non-Hermes target. Hermes targets are skipped so a genuinely slow Hermes
36+
* first-build keeps the full waitForReact budget rather than being aborted.
37+
*/
38+
export function shouldRunPickerProbe(intent, target) {
39+
return intent === 'status' && target.vm !== 'Hermes';
40+
}
41+
export async function autoConnect(ctx, portHint, filters, intent = 'default') {
842
if (ctx.getState() === 'connecting' || ctx.isReconnecting()) {
943
throw new Error('Already connecting to Metro...');
1044
}
@@ -25,12 +59,15 @@ export async function autoConnect(ctx, portHint, filters) {
2559
if (resolved)
2660
effective.preferredBundleId = resolved;
2761
}
28-
return discoverAndConnect(ctx, portHint, effective);
62+
return discoverAndConnect(ctx, portHint, effective, discover, intent);
2963
}
3064
export async function discoverAndConnect(ctx, portHint, filters,
3165
// B111 (D643): injectable for unit tests — defaults to real discover. Production
3266
// call sites pass nothing, so behavior is unchanged. Tests pass a stub.
33-
discoverFn = discover) {
67+
discoverFn = discover,
68+
// GH #184: connect intent threaded to connectToTarget. Kept last so existing
69+
// callers (and tests passing discoverFn as the 4th arg) are unaffected.
70+
intent = 'default') {
3471
if (ctx.isDisposed()) {
3572
throw new Error('Client is disposed. Create a new CDPClient instance.');
3673
}
@@ -71,7 +108,7 @@ discoverFn = discover) {
71108
const candidate = sorted[idx];
72109
const isLast = idx === sorted.length - 1;
73110
try {
74-
await connectToTarget(ctx, candidate);
111+
await connectToTarget(ctx, candidate, 5, intent);
75112
const devCheck = await ctx.evaluate('typeof __DEV__ !== "undefined" && __DEV__ === true');
76113
if (devCheck.value === true) {
77114
connectedTarget = candidate;
@@ -89,6 +126,12 @@ discoverFn = discover) {
89126
connectedTarget = candidate;
90127
}
91128
catch (err) {
129+
// GH #184: picker-blocking affects the whole bundle — every other
130+
// candidate is the same stale C++ target, so don't waste a probe on each.
131+
if (err instanceof PickerBlockingBundleError) {
132+
ctx.setState('disconnected');
133+
throw err;
134+
}
92135
if (!isLast)
93136
continue;
94137
throw err;
@@ -151,7 +194,7 @@ export function formatConnectFailureMessage(retries, attempts, bundleHint, lastE
151194
: '';
152195
return `Failed to connect after ${retries} attempts.${hint}`;
153196
}
154-
async function connectToTarget(ctx, target, retries = 5) {
197+
async function connectToTarget(ctx, target, retries = 5, intent = 'default') {
155198
let lastError = null;
156199
// GH #105 / B154: track per-attempt outcome (handshake ok vs probe timeout).
157200
// Fed into formatConnectFailureMessage at the end.
@@ -187,10 +230,28 @@ async function connectToTarget(ctx, target, retries = 5) {
187230
// M11: stamp connection time so cdp_console_log / cdp_network_log can reason
188231
// about "how long have we been connected with nothing happening?"
189232
ctx.setConnectedAt(ctx.now());
233+
// GH #184: for a status-scoped connect, bounded-probe React reachability
234+
// BEFORE the up-to-30s waitForReact inside setup(). A non-Hermes target
235+
// that can't reach React within the budget is a stale C++ connection the
236+
// Dev Client picker leaves advertised — abort fast with a typed error
237+
// instead of hanging. Hermes targets are skipped (legit slow builds).
238+
if (shouldRunPickerProbe(intent, target)) {
239+
const reachable = await probeReactReachable((expr) => ctx.evaluate(expr), PICKER_PROBE_BUDGET_MS);
240+
if (!reachable)
241+
throw new PickerBlockingBundleError(target);
242+
}
190243
await ctx.setup();
191244
return;
192245
}
193246
catch (err) {
247+
// GH #184: the picker-blocking abort is deterministic, not transient —
248+
// don't burn the retry budget on it; clean up and surface it immediately.
249+
if (err instanceof PickerBlockingBundleError) {
250+
closeAndResetWs(ctx);
251+
ctx.setConnectedTarget(null);
252+
ctx.setState('disconnected');
253+
throw err;
254+
}
194255
lastError = err instanceof Error ? err : new Error(String(err));
195256
attempts.push({ handshakeOk, probeTimedOut });
196257
closeAndResetWs(ctx);

scripts/cdp-bridge/dist/cdp/setup.js

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,28 @@ export async function reinjectHelpers(evaluate, waitTimeout) {
118118
}
119119
return true;
120120
}
121+
/**
122+
* GH #184: bounded variant of waitForReact that RETURNS whether React became
123+
* reachable within `budgetMs` (vs waitForReact which always resolves void after
124+
* logging). Used by the status-scoped picker-blocking probe to decide fast,
125+
* before the full REACT_READY_TIMEOUT_MS wait in setup(), whether a non-Hermes
126+
* target is a stale (picker-blocked) connection.
127+
*/
128+
export async function probeReactReachable(evaluate, budgetMs, pollMs = REACT_READY_POLL_MS) {
129+
const start = Date.now();
130+
while (Date.now() - start < budgetMs) {
131+
try {
132+
const result = await evaluate(REACT_READY_PROBE_JS);
133+
if (result.value === true)
134+
return true;
135+
}
136+
catch {
137+
// not ready yet
138+
}
139+
await sleep(pollMs);
140+
}
141+
return false;
142+
}
121143
export async function waitForReact(evaluate, timeout, pollInterval) {
122144
const effectiveTimeout = timeout ?? REACT_READY_TIMEOUT_MS;
123145
const effectivePoll = pollInterval ?? REACT_READY_POLL_MS;

scripts/cdp-bridge/dist/tools/status.js

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import { okResult, failResult, warnResult } from '../utils.js';
22
import { handleDevClientPicker, isDevClientPickerShowing } from './dev-client-picker.js';
3+
import { PickerBlockingBundleError } from '../cdp/connect.js';
34
import { getSessionReloadCount } from './reload.js';
45
import { supportsNativeMultiDebugger } from '../cdp/multiplexer.js';
56
// M10 / Phase 110: narrow `appInfo.architecture` to the StatusResult union.
@@ -111,7 +112,7 @@ export function createStatusHandler(getClient, setClient, createClient) {
111112
}
112113
}
113114
catch { /* fall through to autoConnect */ }
114-
await client.autoConnect(args.metroPort, args.platform);
115+
await client.autoConnect(args.metroPort, args.platform, 'status');
115116
}
116117
else if (args.platform) {
117118
// GH #21: Already connected — check if the current target matches the requested platform
@@ -123,7 +124,7 @@ export function createStatusHandler(getClient, setClient, createClient) {
123124
await client.disconnect();
124125
client = createClient(client.metroPort);
125126
setClient(client);
126-
await client.autoConnect(args.metroPort, args.platform);
127+
await client.autoConnect(args.metroPort, args.platform, 'status');
127128
}
128129
}
129130
const status = await buildStatusResult(client);
@@ -203,6 +204,12 @@ export function createStatusHandler(getClient, setClient, createClient) {
203204
}
204205
catch (err) {
205206
const message = err instanceof Error ? err.message : String(err);
207+
// GH #184: the status-scoped connect aborted fast because React was
208+
// unreachable on a non-Hermes target (picker blocking the bundle, or it's
209+
// still building). The message is already actionable; still attempt the
210+
// best-effort auto-dismiss below (helps when a device session is open),
211+
// then surface it with a typed code instead of a generic failure.
212+
const pickerBlocking = err instanceof PickerBlockingBundleError;
206213
// If connection failed, check if the Dev Client picker is blocking
207214
try {
208215
const pickerResult = await handleDevClientPicker();
@@ -227,7 +234,7 @@ export function createStatusHandler(getClient, setClient, createClient) {
227234
}
228235
}
229236
catch { /* picker check failed, return original error */ }
230-
return failResult(message);
237+
return pickerBlocking ? failResult(message, 'PICKER_BLOCKING') : failResult(message);
231238
}
232239
};
233240
}

scripts/cdp-bridge/src/cdp-client.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import {
1919
autoConnect as autoConnectFn,
2020
discoverAndConnect as discoverAndConnectFn,
2121
} from './cdp/connect.js';
22-
import type { ConnectContext, ConnectFilters } from './cdp/connect.js';
22+
import type { ConnectContext, ConnectFilters, ConnectIntent } from './cdp/connect.js';
2323
import {
2424
handleClose as handleCloseFn,
2525
reconnect as reconnectFn,
@@ -167,11 +167,11 @@ export class CDPClient {
167167
return ok;
168168
}
169169

170-
async autoConnect(portHint?: number, filtersOrPlatform?: string | ConnectFilters): Promise<string> {
170+
async autoConnect(portHint?: number, filtersOrPlatform?: string | ConnectFilters, intent: ConnectIntent = 'default'): Promise<string> {
171171
const filters: ConnectFilters = typeof filtersOrPlatform === 'string'
172172
? { platform: filtersOrPlatform }
173173
: (filtersOrPlatform ?? {});
174-
return autoConnectFn(this.buildConnectCtx(), portHint, filters);
174+
return autoConnectFn(this.buildConnectCtx(), portHint, filters, intent);
175175
}
176176

177177
async listTargets(portHint?: number): Promise<{ port: number; targets: HermesTarget[] }> {

scripts/cdp-bridge/src/cdp/connect.ts

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,52 @@ import { discover } from './discovery.js';
55
import type { SelectTargetFilters } from './discovery.js';
66
import { sleep } from './state.js';
77
import { CDP_TIMEOUT_FAST } from './timeout-config.js';
8+
import { probeReactReachable } from './setup.js';
89
import type { CDPClientState, EvaluateResult, HermesTarget } from '../types.js';
910

11+
// GH #184: distinguishes a quick health-check connect (cdp_status) from every
12+
// other connect path. Only 'status' connects run the bounded picker probe; all
13+
// other paths keep the full waitForReact budget for legitimate slow builds.
14+
export type ConnectIntent = 'default' | 'status';
15+
16+
// Budget for the status-scoped React-reachability probe. Generous enough to
17+
// clear a normal Bridgeless reload (React ready in <~2s) but far below setup()'s
18+
// 30s waitForReact, so cdp_status fails fast instead of hanging when the Dev
19+
// Client picker blocks the bundle. Env-overridable for tuning/tests.
20+
const PICKER_PROBE_BUDGET_MS = (() => {
21+
const n = parseInt(process.env.RN_PICKER_PROBE_BUDGET_MS ?? '', 10);
22+
return Number.isFinite(n) && n > 0 ? n : 5000;
23+
})();
24+
25+
/**
26+
* GH #184: thrown when a status-scoped connect can't reach React within the
27+
* bounded budget on a non-Hermes target — the signature of the Expo Dev Client
28+
* "Development servers" picker leaving stale C++ targets advertised while the
29+
* bundle never loads. status.ts maps it to a fast, actionable failResult
30+
* instead of letting setup() burn the full 30s waitForReact.
31+
*/
32+
export class PickerBlockingBundleError extends Error {
33+
readonly target: HermesTarget;
34+
constructor(target: HermesTarget) {
35+
super(
36+
`Dev Client picker appears to be blocking the bundle: React was not reachable on target ` +
37+
`"${target.title}" (vm=${target.vm}). If the Expo "Development servers" picker is showing on ` +
38+
`the simulator, select your Metro server, then retry cdp_status. (If the bundle is still building, just retry.)`,
39+
);
40+
this.name = 'PickerBlockingBundleError';
41+
this.target = target;
42+
}
43+
}
44+
45+
/**
46+
* GH #184: run the bounded picker probe only for a status-intent connect against
47+
* a non-Hermes target. Hermes targets are skipped so a genuinely slow Hermes
48+
* first-build keeps the full waitForReact budget rather than being aborted.
49+
*/
50+
export function shouldRunPickerProbe(intent: ConnectIntent, target: HermesTarget): boolean {
51+
return intent === 'status' && target.vm !== 'Hermes';
52+
}
53+
1054
export interface ConnectFilters {
1155
platform?: string;
1256
targetId?: string;
@@ -50,6 +94,7 @@ export async function autoConnect(
5094
ctx: ConnectContext,
5195
portHint?: number,
5296
filters?: ConnectFilters,
97+
intent: ConnectIntent = 'default',
5398
): Promise<string> {
5499
if (ctx.getState() === 'connecting' || ctx.isReconnecting()) {
55100
throw new Error('Already connecting to Metro...');
@@ -69,7 +114,7 @@ export async function autoConnect(
69114
const resolved = resolveBundleId(effective.platform ?? 'ios');
70115
if (resolved) effective.preferredBundleId = resolved;
71116
}
72-
return discoverAndConnect(ctx, portHint, effective);
117+
return discoverAndConnect(ctx, portHint, effective, discover, intent);
73118
}
74119

75120
export async function discoverAndConnect(
@@ -79,6 +124,9 @@ export async function discoverAndConnect(
79124
// B111 (D643): injectable for unit tests — defaults to real discover. Production
80125
// call sites pass nothing, so behavior is unchanged. Tests pass a stub.
81126
discoverFn: typeof discover = discover,
127+
// GH #184: connect intent threaded to connectToTarget. Kept last so existing
128+
// callers (and tests passing discoverFn as the 4th arg) are unaffected.
129+
intent: ConnectIntent = 'default',
82130
): Promise<string> {
83131
if (ctx.isDisposed()) {
84132
throw new Error('Client is disposed. Create a new CDPClient instance.');
@@ -123,7 +171,7 @@ export async function discoverAndConnect(
123171
const candidate = sorted[idx];
124172
const isLast = idx === sorted.length - 1;
125173
try {
126-
await connectToTarget(ctx, candidate);
174+
await connectToTarget(ctx, candidate, 5, intent);
127175
const devCheck = await ctx.evaluate('typeof __DEV__ !== "undefined" && __DEV__ === true');
128176
if (devCheck.value === true) {
129177
connectedTarget = candidate;
@@ -140,6 +188,12 @@ export async function discoverAndConnect(
140188
console.error('CDP: no target with __DEV__=true found, using last available target');
141189
connectedTarget = candidate;
142190
} catch (err) {
191+
// GH #184: picker-blocking affects the whole bundle — every other
192+
// candidate is the same stale C++ target, so don't waste a probe on each.
193+
if (err instanceof PickerBlockingBundleError) {
194+
ctx.setState('disconnected');
195+
throw err;
196+
}
143197
if (!isLast) continue;
144198
throw err;
145199
}
@@ -218,6 +272,7 @@ async function connectToTarget(
218272
ctx: ConnectContext,
219273
target: HermesTarget,
220274
retries = 5,
275+
intent: ConnectIntent = 'default',
221276
): Promise<void> {
222277
let lastError: Error | null = null;
223278
// GH #105 / B154: track per-attempt outcome (handshake ok vs probe timeout).
@@ -253,9 +308,26 @@ async function connectToTarget(
253308
// M11: stamp connection time so cdp_console_log / cdp_network_log can reason
254309
// about "how long have we been connected with nothing happening?"
255310
ctx.setConnectedAt(ctx.now());
311+
// GH #184: for a status-scoped connect, bounded-probe React reachability
312+
// BEFORE the up-to-30s waitForReact inside setup(). A non-Hermes target
313+
// that can't reach React within the budget is a stale C++ connection the
314+
// Dev Client picker leaves advertised — abort fast with a typed error
315+
// instead of hanging. Hermes targets are skipped (legit slow builds).
316+
if (shouldRunPickerProbe(intent, target)) {
317+
const reachable = await probeReactReachable((expr) => ctx.evaluate(expr), PICKER_PROBE_BUDGET_MS);
318+
if (!reachable) throw new PickerBlockingBundleError(target);
319+
}
256320
await ctx.setup();
257321
return;
258322
} catch (err) {
323+
// GH #184: the picker-blocking abort is deterministic, not transient —
324+
// don't burn the retry budget on it; clean up and surface it immediately.
325+
if (err instanceof PickerBlockingBundleError) {
326+
closeAndResetWs(ctx);
327+
ctx.setConnectedTarget(null);
328+
ctx.setState('disconnected');
329+
throw err;
330+
}
259331
lastError = err instanceof Error ? err : new Error(String(err));
260332
attempts.push({ handshakeOk, probeTimedOut });
261333
closeAndResetWs(ctx);

0 commit comments

Comments
 (0)