Skip to content

Commit 8783e1e

Browse files
adamzielmho22
andauthored
[XDebug Bridge] Fetch all array keys when inspecting an array (#2409)
## Motivation for the change, related issues Fixes two problems: * Fetches all the array keys when inspecting arrays. `property_get` XDebug command is paginated and before this PR, we'd only see a subset of all the array keys. With this PR, we see all the keys. This will be a problem on huge arrays so we'll need to bucket keys into group of 1000 in a follow-up PR. * Supports actually browsing superglobals, e.g. $_SERVER. Before this PR, you'd only see another nested superglobal (see the screenshot below). As for the latter, I'm not too happy about the solution – we're reaching for the nested XDebug `.property.property` when it's available. I expect that will lead us astray in some scenarios, e.g. when browsing arrays that have nested arrays. That being said, I couldn't easily break it so let's get it in and continue iterating: ```ts const responseProps = response.property?.property ?? response.property; ``` ## Testing instructions Create a file called `xdebug.php`: ```php <?php unset($_SERVER['CLIENT_SECRET'], $_SERVER['CLIENT_ID']); $test = 42; // Set a breakpoint on this line $another = [15, [1,2,3, [4,5,6] ] ]; echo "Output!\n"; function test() { echo "Hello Xdebug World!\n"; print_R($_ENV); print_r($_SERVER); } test(); ``` Run `nx run php-wasm-cli:dev --xdebug --experimental-devtools xdebug.php` before and after this PR. Confirm you see the following results: **Before** <img width="660" alt="CleanShot 2025-07-24 at 00 33 54@2x" src="https://github.com/user-attachments/assets/fdbc8ac2-9168-44ee-926a-b95416c9970b" /> **After** ($_SERVER is still duplicated – let's fix that separately) <img width="537" alt="CleanShot 2025-07-24 at 00 32 50@2x" src="https://github.com/user-attachments/assets/28633976-714a-423f-90e6-b65207e56e26" /> --------- Co-authored-by: mho22 <[email protected]>
1 parent b195d93 commit 8783e1e

15 files changed

+914
-25
lines changed

packages/php-wasm/xdebug-bridge/project.json

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,8 +67,7 @@
6767
"{workspaceRoot}/coverage/packages/php-wasm/xdebug-bridge"
6868
],
6969
"options": {
70-
"reportsDirectory": "../../../coverage/packages/php-wasm/xdebug-bridge",
71-
"testFiles": ["mock-test.spec.ts"]
70+
"reportsDirectory": "../../../coverage/packages/php-wasm/xdebug-bridge"
7271
}
7372
},
7473
"typecheck": {

packages/php-wasm/xdebug-bridge/src/lib/cdp-server.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,4 +47,8 @@ export class CDPServer extends EventEmitter {
4747
console.log('\x1b[1;32m[CDP][send]\x1b[0m', json);
4848
this.ws.send(json);
4949
}
50+
51+
close() {
52+
this.wss.close();
53+
}
5054
}

packages/php-wasm/xdebug-bridge/src/lib/dbgp-session.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,4 +80,8 @@ export class DbgpSession extends EventEmitter {
8080
// Commands must end with null terminator
8181
this.socket.write(command + '\x00');
8282
}
83+
84+
close() {
85+
this.server.close();
86+
}
8387
}

packages/php-wasm/xdebug-bridge/src/lib/xdebug-cdp-bridge.ts

Lines changed: 115 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ interface ObjectHandle {
2222
contextId?: number;
2323
depth: number;
2424
fullname?: string;
25+
// Add pagination support
26+
currentPage?: number;
27+
totalPages?: number;
28+
aggregatedProps?: any[];
2529
}
2630

2731
export interface XdebugCDPBridgeConfig {
@@ -136,6 +140,11 @@ export class XdebugCDPBridge {
136140
});
137141
}
138142

143+
stop() {
144+
this.dbgp.close();
145+
this.cdp.close();
146+
}
147+
139148
private sendInitialScripts() {
140149
// Send scriptParsed for the main file if not already sent
141150
if (this.initFileUri && !this.scriptIdByUrl.has(this.initFileUri)) {
@@ -387,20 +396,36 @@ export class XdebugCDPBridge {
387396
if (handle.type === 'context') {
388397
const contextId = handle.contextId ?? 0;
389398
const depth = handle.depth;
390-
// Get variables in the context
391-
const cmd = `context_get -d ${depth} -c ${contextId}`;
399+
// Get variables in the context with pagination support (32 items per page)
400+
const cmd = `context_get -d ${depth} -c ${contextId} -p 0 -m 32`;
392401
const txn = this.sendDbgpCommand(cmd);
402+
// Initialize pagination state
403+
const updatedHandle = {
404+
...handle,
405+
currentPage: 0,
406+
aggregatedProps: [],
407+
};
408+
this.objectHandles.set(objectId, updatedHandle);
393409
this.pendingCommands.set(txn, {
394410
cdpId: id,
395411
cdpMethod: method,
412+
params: { objectId: objectId },
396413
});
397414
sendResponse = false;
398415
} else if (handle.type === 'property') {
399416
const depth = handle.depth;
400417
const fullname = handle.fullname!;
401418
const fmtName = this.formatPropertyFullName(fullname);
402-
const cmd = `property_get -d ${depth} -n ${fmtName}`;
419+
// Get property with pagination support (32 items per page)
420+
const cmd = `property_get -d ${depth} -n ${fmtName} -p 0 -m 32`;
403421
const txn = this.sendDbgpCommand(cmd);
422+
// Initialize pagination state
423+
const updatedHandle = {
424+
...handle,
425+
currentPage: 0,
426+
aggregatedProps: [],
427+
};
428+
this.objectHandles.set(objectId, updatedHandle);
404429
this.pendingCommands.set(txn, {
405430
cdpId: id,
406431
cdpMethod: method,
@@ -686,13 +711,26 @@ export class XdebugCDPBridge {
686711
case 'context_get':
687712
case 'property_get': {
688713
if (pending && pending.cdpId !== undefined) {
689-
// Handle variables or object properties retrieval
690-
const props: any = [];
691-
const responseProps = response.property;
714+
// Handle variables or object properties retrieval with pagination
715+
const objectId =
716+
pending.params?.objectId ||
717+
pending.params?.parentObjectId;
718+
const handle = objectId
719+
? this.objectHandles.get(objectId)
720+
: null;
721+
722+
// @TODO: This is hacky. It enables browsing arrays. Without it,
723+
// the debugger shows $_SERVER as an array with a single property called
724+
// $_SERVER.
725+
const responseProps =
726+
response.property?.property ?? response.property;
727+
728+
const currentProps: any[] = [];
692729
if (responseProps) {
693730
const propertiesArray = Array.isArray(responseProps)
694731
? responseProps
695732
: [responseProps];
733+
696734
for (const prop of propertiesArray) {
697735
const name =
698736
prop.$.name || prop.$.fullname || '';
@@ -722,7 +760,7 @@ export class XdebugCDPBridge {
722760
const className =
723761
prop.$.classname ||
724762
(type === 'array' ? 'Array' : 'Object');
725-
const objectId = String(
763+
const childObjectId = String(
726764
this.nextObjectId++
727765
);
728766
// Store handle
@@ -745,19 +783,20 @@ export class XdebugCDPBridge {
745783
)?.depth || 0
746784
: 0;
747785
// Use same depth/context as parent
748-
this.objectHandles.set(objectId, {
786+
this.objectHandles.set(childObjectId, {
749787
type: 'property',
750788
depth: depth,
751789
contextId: contextId,
752790
fullname: prop.$.fullname || name,
753791
});
754-
props.push({
792+
793+
currentProps.push({
755794
name: prop.$.key || name,
756795
value: {
757796
type: 'object',
758797
className: className,
759798
description: className,
760-
objectId: objectId,
799+
objectId: childObjectId,
761800
},
762801
writable: false,
763802
configurable: false,
@@ -802,7 +841,7 @@ export class XdebugCDPBridge {
802841
};
803842
if (subtype) valueObj.subtype = subtype;
804843
valueObj.value = value;
805-
props.push({
844+
currentProps.push({
806845
name: prop.$.key || name,
807846
value: valueObj,
808847
writable: false,
@@ -812,9 +851,71 @@ export class XdebugCDPBridge {
812851
}
813852
}
814853
}
815-
const result = { result: props };
816-
this.cdp.sendMessage({ id: pending.cdpId, result });
817-
this.pendingCommands.delete(transId);
854+
855+
// Handle pagination
856+
if (handle) {
857+
// Add current page props to aggregated results
858+
const aggregatedProps = (
859+
handle.aggregatedProps || []
860+
).concat(currentProps);
861+
862+
// Check if there are more pages - if we got exactly 32 items (page size), there might be more
863+
const pageSize = 32;
864+
const hasMorePages =
865+
currentProps.length === pageSize;
866+
867+
if (hasMorePages) {
868+
// More pages available, fetch next page
869+
const nextPage = (handle.currentPage || 0) + 1;
870+
const updatedHandle = {
871+
...handle,
872+
currentPage: nextPage,
873+
aggregatedProps: aggregatedProps,
874+
};
875+
this.objectHandles.set(
876+
objectId!,
877+
updatedHandle
878+
);
879+
880+
// Send command for next page
881+
let nextCmd: string;
882+
if (command === 'context_get') {
883+
const contextId = handle.contextId ?? 0;
884+
const depth = handle.depth;
885+
nextCmd = `context_get -d ${depth} -c ${contextId} -p ${nextPage} -m ${pageSize}`;
886+
} else {
887+
// property_get
888+
const depth = handle.depth;
889+
const fullname = handle.fullname!;
890+
const fmtName =
891+
this.formatPropertyFullName(fullname);
892+
nextCmd = `property_get -d ${depth} -n ${fmtName} -p ${nextPage} -m ${pageSize}`;
893+
}
894+
895+
const txn = this.sendDbgpCommand(nextCmd);
896+
this.pendingCommands.set(txn, {
897+
cdpId: pending.cdpId,
898+
cdpMethod: pending.cdpMethod,
899+
params: pending.params,
900+
});
901+
// Don't send response yet, wait for more pages
902+
this.pendingCommands.delete(transId);
903+
return;
904+
} else {
905+
// No more pages or last page, send final response
906+
const result = { result: aggregatedProps };
907+
this.cdp.sendMessage({
908+
id: pending.cdpId,
909+
result,
910+
});
911+
this.pendingCommands.delete(transId);
912+
}
913+
} else {
914+
// No handle, send current props
915+
const result = { result: currentProps };
916+
this.cdp.sendMessage({ id: pending.cdpId, result });
917+
this.pendingCommands.delete(transId);
918+
}
818919
}
819920
break;
820921
}
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
import './mocker';
2+
import { vi } from 'vitest';
3+
import { WebSocket } from 'ws';
4+
import { CDPServer } from '../lib/cdp-server';
5+
6+
describe('CDPServer', () => {
7+
let server: any;
8+
let socket: any;
9+
10+
beforeEach(() => {
11+
// @ts-ignore
12+
socket = new WebSocket();
13+
server = new CDPServer(9999);
14+
});
15+
16+
afterEach(() => {
17+
server.removeAllListeners();
18+
vi.clearAllMocks();
19+
});
20+
21+
it('emits clientConnected on new connection', () => {
22+
const onClientConnected = vi.fn();
23+
24+
server.on('clientConnected', onClientConnected);
25+
server.wss.emit('connection', socket);
26+
27+
expect(onClientConnected).toHaveBeenCalled();
28+
});
29+
30+
it('only allows one client at a time', () => {
31+
// @ts-ignore
32+
const client = new WebSocket();
33+
const spy = vi.spyOn(client, 'close');
34+
35+
server.wss.emit('connection', socket);
36+
server.wss.emit('connection', client);
37+
38+
expect(spy).toHaveBeenCalledTimes(1);
39+
});
40+
41+
it('emits message when a valid JSON message is received', () => {
42+
const msg = { id: 1, method: 'Debugger.enable' };
43+
const onMessage = vi.fn();
44+
45+
server.on('message', onMessage);
46+
server.wss.emit('connection', socket);
47+
socket.emit('message', JSON.stringify(msg));
48+
49+
expect(onMessage).toHaveBeenCalledWith(msg);
50+
});
51+
52+
it('ignores invalid JSON messages', () => {
53+
const onMessage = vi.fn();
54+
55+
server.on('message', onMessage);
56+
server.wss.emit('connection', socket);
57+
socket.emit('message', '{ invalid json }');
58+
59+
expect(onMessage).not.toHaveBeenCalled();
60+
});
61+
62+
it('emits clientDisconnected when client closes', () => {
63+
const onDisconnect = vi.fn();
64+
65+
server.on('clientDisconnected', onDisconnect);
66+
server.wss.emit('connection', socket);
67+
socket.emit('close');
68+
69+
expect(onDisconnect).toHaveBeenCalled();
70+
});
71+
72+
it('emits error on websocket error', () => {
73+
const error = new Error('Test error');
74+
const onError = vi.fn();
75+
76+
server.on('error', onError);
77+
server.wss.emit('connection', socket);
78+
socket.emit('error', error);
79+
80+
expect(onError).toHaveBeenCalledWith(error);
81+
});
82+
83+
it('sends a message if client is connected and open', () => {
84+
const msg = { id: 1, result: 'ok' };
85+
86+
server.wss.emit('connection', socket);
87+
server.sendMessage(msg);
88+
89+
expect(socket.send).toHaveBeenCalledWith(JSON.stringify(msg));
90+
});
91+
92+
it('does not send message if no client connected', () => {
93+
server.sendMessage({ hello: 'world' });
94+
95+
expect(socket.send).not.toHaveBeenCalled();
96+
});
97+
98+
it('does not send message if client is not OPEN', () => {
99+
socket.readyState = 0;
100+
101+
server.wss.emit('connection', socket);
102+
server.sendMessage({ id: 1 });
103+
104+
expect(socket.send).not.toHaveBeenCalled();
105+
});
106+
});

0 commit comments

Comments
 (0)