Skip to content

Commit fad02f7

Browse files
committed
Simplify server upstream selection
1 parent db2c182 commit fad02f7

11 files changed

Lines changed: 101 additions & 100 deletions

go/modcdp/client/ModCDPClient.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -831,12 +831,7 @@ func (c *ModCDPClient) serverNeedsLoopbackCDP() bool {
831831
if c.Server == nil || c.Server.ServerLoopbackCDPURL != "" {
832832
return false
833833
}
834-
for _, route := range c.Server.ServerRoutes {
835-
if route == "loopback_cdp" {
836-
return true
837-
}
838-
}
839-
return false
834+
return c.Server.ServerRoutes["*.*"] == "loopback_cdp"
840835
}
841836

842837
func (c *ModCDPClient) ensureModCDPServerConfigured() error {

go/modcdp/client/ModCDPClientRoutedDefaultOverrides_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ import (
99
const getTargetsOverride = `
1010
async (params) => {
1111
const [upstream, tabs] = await Promise.all([
12-
ModCDP.upstream.send("Target.getTargets", params),
12+
cdp.upstream.send("Target.getTargets", params),
1313
chrome.tabs.query({}),
1414
]);
1515

go/modcdp/injector/extension.zip

-207 Bytes
Binary file not shown.

js/src/client/ModCDPClient.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -497,10 +497,10 @@ export class ModCDPClient extends ModCDPEventEmitter {
497497
};
498498
},
499499
};
500-
this.auto_sessions = new AutoSessionRouter(
501-
raw_upstream_transport,
502-
() => this.injector.injector_execution_context_timeout_ms,
503-
);
500+
this.auto_sessions = new AutoSessionRouter({
501+
upstream: raw_upstream_transport,
502+
loopback_execution_context_timeout_ms: this.injector.injector_execution_context_timeout_ms,
503+
});
504504
this.auto_sessions.listen();
505505
this._injectors = [];
506506
this._launched = null;
@@ -952,7 +952,7 @@ export class ModCDPClient extends ModCDPEventEmitter {
952952

953953
_serverNeedsLoopbackCdp() {
954954
if (!this.server || this.server.server_loopback_cdp_url) return false;
955-
return Object.values(this.server.server_routes ?? {}).includes("loopback_cdp");
955+
return this.server.server_routes?.["*.*"] === "loopback_cdp";
956956
}
957957

958958
_upstreamTransportConfig() {

js/src/router/AutoSessionRouter.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,13 +94,20 @@ export class AutoSessionRouter {
9494
// object but never mutates transport-owned private state.
9595
private readonly upstream: ServerUpstreamTransport;
9696

97-
// Timeout supplier owned by client/server config. Read when installing a new
98-
// Runtime.executionContextCreated waiter.
99-
private readonly defaultExecutionContextTimeoutMs: () => number;
100-
101-
constructor(upstream: ServerUpstreamTransport, defaultExecutionContextTimeoutMs: () => number) {
97+
// Timeout in milliseconds for Runtime.executionContextCreated waits. Set once
98+
// by the owner when constructing the router; read when installing a new
99+
// execution-context waiter.
100+
private readonly loopback_execution_context_timeout_ms: number;
101+
102+
constructor({
103+
upstream,
104+
loopback_execution_context_timeout_ms,
105+
}: {
106+
upstream: ServerUpstreamTransport;
107+
loopback_execution_context_timeout_ms: number;
108+
}) {
102109
this.upstream = upstream;
103-
this.defaultExecutionContextTimeoutMs = defaultExecutionContextTimeoutMs;
110+
this.loopback_execution_context_timeout_ms = loopback_execution_context_timeout_ms;
104111
}
105112

106113
/** Route a CDP command using router-owned target/session policy. */
@@ -521,7 +528,7 @@ export class AutoSessionRouter {
521528
private waitForExecutionContextMatching(
522529
matches: (context: ModCDPTopologyExecutionContext) => boolean,
523530
waiterKey: string | null,
524-
timeoutMs = this.defaultExecutionContextTimeoutMs(),
531+
timeoutMs = this.loopback_execution_context_timeout_ms,
525532
): Promise<ModCDPTopologyExecutionContext> {
526533
for (const context of this.contexts.values()) {
527534
if (matches(context)) return Promise.resolve(context);

js/src/server/LoopbackCdpTransport.ts

Lines changed: 35 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,10 @@ import {
1414
import type { ServerUpstreamEventListener, ServerUpstreamTransport, TargetRoute } from "./ServerUpstreamTransport.js";
1515

1616
type LoopbackCdpTransportOptions = {
17-
getLoopbackCdpUrl: () => string | null;
18-
setLoopbackCdpUrl: (url: string | null) => void;
19-
getCdpSendTimeoutMs: () => number;
20-
getExecutionContextTimeoutMs: () => number;
21-
getWsConnectErrorSettleTimeoutMs: () => number;
17+
loopback_cdp_url: string | null;
18+
cdp_send_timeout_ms: number;
19+
loopback_execution_context_timeout_ms: number;
20+
ws_connect_error_settle_timeout_ms: number;
2221
};
2322

2423
const target_auto_attach_params = {
@@ -38,7 +37,7 @@ const target_auto_attach_params = {
3837
* discovery probe needed to verify the current service worker.
3938
*
4039
* Lifecycle:
41-
* 1. The server constructs the transport with config accessors.
40+
* 1. The server constructs the transport with current config values.
4241
* 2. `getTargets()` or `send()` opens the loopback WebSocket and initializes
4342
* target auto-attach/discovery once per socket.
4443
* 3. CDP socket event messages are normalized and dispatched to typed
@@ -84,20 +83,27 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
8483
// emitLoopbackUpstreamEvent from the WebSocket message handler.
8584
private readonly event_listeners = new Map<CdpNamedSchema<z.ZodType>, Set<ServerUpstreamEventListener>>();
8685

87-
// Runtime/config accessors owned by ModCDPServer. Read whenever a command or
88-
// socket operation needs current config values.
89-
private readonly getLoopbackCdpUrl: LoopbackCdpTransportOptions["getLoopbackCdpUrl"];
90-
private readonly setLoopbackCdpUrl: LoopbackCdpTransportOptions["setLoopbackCdpUrl"];
91-
private readonly getCdpSendTimeoutMs: LoopbackCdpTransportOptions["getCdpSendTimeoutMs"];
92-
private readonly getExecutionContextTimeoutMs: LoopbackCdpTransportOptions["getExecutionContextTimeoutMs"];
93-
private readonly getWsConnectErrorSettleTimeoutMs: LoopbackCdpTransportOptions["getWsConnectErrorSettleTimeoutMs"];
86+
// Current loopback CDP endpoint owned by this transport instance. Written by
87+
// discoverLoopbackCDP while probing; read by all loopback CDP sends.
88+
private loopback_cdp_url: string | null;
89+
90+
// Request timeout for loopback CDP sends. Set at construction from server
91+
// config; read by sendToLoopback.
92+
private readonly cdp_send_timeout_ms: number;
93+
94+
// Runtime.executionContextCreated wait timeout for discovery. Set at
95+
// construction from server config; read by waitForLoopbackExecutionContext.
96+
private readonly loopback_execution_context_timeout_ms: number;
97+
98+
// Delay used to let websocket close details arrive after an error event. Set
99+
// at construction from server config; read by loopbackWS.
100+
private readonly ws_connect_error_settle_timeout_ms: number;
94101

95102
constructor(options: LoopbackCdpTransportOptions) {
96-
this.getLoopbackCdpUrl = options.getLoopbackCdpUrl;
97-
this.setLoopbackCdpUrl = options.setLoopbackCdpUrl;
98-
this.getCdpSendTimeoutMs = options.getCdpSendTimeoutMs;
99-
this.getExecutionContextTimeoutMs = options.getExecutionContextTimeoutMs;
100-
this.getWsConnectErrorSettleTimeoutMs = options.getWsConnectErrorSettleTimeoutMs;
103+
this.loopback_cdp_url = options.loopback_cdp_url;
104+
this.cdp_send_timeout_ms = options.cdp_send_timeout_ms;
105+
this.loopback_execution_context_timeout_ms = options.loopback_execution_context_timeout_ms;
106+
this.ws_connect_error_settle_timeout_ms = options.ws_connect_error_settle_timeout_ms;
101107
this.on(Runtime.ExecutionContextCreatedEvent, (event, _targetId, sessionId) => {
102108
if (sessionId == null) return;
103109
this.loopback_session_contexts.set(sessionId, event.context.id);
@@ -145,7 +151,7 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
145151

146152
/** Return current browser targets through the loopback CDP endpoint. */
147153
async getTargets() {
148-
if (!this.getLoopbackCdpUrl()) throw new Error(`No loopback_cdp_url configured for Target.getTargets.`);
154+
if (!this.loopback_cdp_url) throw new Error(`No loopback_cdp_url configured for Target.getTargets.`);
149155
await this.initializeLoopbackCDP();
150156
return (await this.send(Target.GetTargetsCommand, {})).targetInfos;
151157
}
@@ -213,9 +219,9 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
213219
if (!browserToken) return { loopback_cdp_url: null as null, verified: false };
214220

215221
const url = "http://127.0.0.1:9222";
216-
const previous_loopback_url = this.getLoopbackCdpUrl();
222+
const previous_loopback_url = this.loopback_cdp_url;
217223
const fail = (version?: unknown) => {
218-
this.setLoopbackCdpUrl(previous_loopback_url ?? null);
224+
this.loopback_cdp_url = previous_loopback_url ?? null;
219225
return {
220226
loopback_cdp_url: null as null,
221227
verified: false,
@@ -226,7 +232,7 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
226232
const version = await fetch(`${url}/json/version`).then((response) => response.ok && response.json());
227233
if (!version?.webSocketDebuggerUrl) return fail();
228234

229-
this.setLoopbackCdpUrl(version.webSocketDebuggerUrl);
235+
this.loopback_cdp_url = version.webSocketDebuggerUrl;
230236
const { targetInfos } = Target.GetTargetsCommand.result.parse(
231237
await this.sendToLoopback(Target.GetTargetsCommand.id, Target.GetTargetsCommand.params.parse({})),
232238
);
@@ -260,7 +266,7 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
260266

261267
await this.initializeLoopbackCDP();
262268
return {
263-
loopback_cdp_url: this.getLoopbackCdpUrl(),
269+
loopback_cdp_url: this.loopback_cdp_url,
264270
verified: true,
265271
version,
266272
};
@@ -375,7 +381,7 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
375381
"error",
376382
(event) => {
377383
error_event = event;
378-
setTimeout(() => fail(new Error(describe("CDP socket error"))), this.getWsConnectErrorSettleTimeoutMs());
384+
setTimeout(() => fail(new Error(describe("CDP socket error"))), this.ws_connect_error_settle_timeout_ms);
379385
},
380386
{ once: true },
381387
);
@@ -384,7 +390,7 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
384390
}
385391

386392
private async sendToLoopback(method: string, params: ProtocolParams = {}, sessionId: string | null = null) {
387-
const endpoint = this.getLoopbackCdpUrl();
393+
const endpoint = this.loopback_cdp_url;
388394
if (!endpoint) throw new Error(`No loopback_cdp_url configured for ${method}.`);
389395
const ws = await this.loopbackWS(endpoint);
390396
const id = this.next_loopback_id++;
@@ -403,8 +409,8 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
403409
return new Promise<ProtocolResult>((resolve, reject) => {
404410
const timeout = setTimeout(() => {
405411
if (!this.loopback_pending.delete(id)) return;
406-
reject(new Error(`${method} timed out after ${this.getCdpSendTimeoutMs()}ms`));
407-
}, this.getCdpSendTimeoutMs());
412+
reject(new Error(`${method} timed out after ${this.cdp_send_timeout_ms}ms`));
413+
}, this.cdp_send_timeout_ms);
408414
this.loopback_pending.set(id, {
409415
resolve: (value) => {
410416
clearTimeout(timeout);
@@ -419,7 +425,7 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
419425
}
420426

421427
private async initializeLoopbackCDP() {
422-
const endpoint = this.getLoopbackCdpUrl();
428+
const endpoint = this.loopback_cdp_url;
423429
if (!endpoint) return;
424430
const ws = await this.loopbackWS(endpoint);
425431
if (this.initialized_loopback_sockets.has(ws)) return;
@@ -434,7 +440,7 @@ export class LoopbackCdpTransport implements ServerUpstreamTransport {
434440
this.initialized_loopback_sockets.add(ws);
435441
}
436442

437-
private waitForLoopbackExecutionContext(sessionId: string, timeout_ms = this.getExecutionContextTimeoutMs()) {
443+
private waitForLoopbackExecutionContext(sessionId: string, timeout_ms = this.loopback_execution_context_timeout_ms) {
438444
const existing = this.loopback_session_contexts.get(sessionId);
439445
if (existing != null) return Promise.resolve(existing);
440446
return new Promise<number>((resolve, reject) => {

0 commit comments

Comments
 (0)