Skip to content

Commit 4c6bff0

Browse files
motiz88facebook-github-bot
authored andcommitted
Fix bug in device ID collision handling, add tests (facebook#45010)
Summary: Pull Request resolved: facebook#45010 D46482492 added logic for handing off state across "device" connections that have the same ID. This logic currently has no test coverage. It also contains a bug whereby the new device's pages are removed from the target listing endpoint (`/json`) when the *old* device's socket is closed. This diff adds tests and fixes the bug. Changelog: [General][Fixed] inspector-proxy no longer accidentally detaches connected devices. ## Next steps It seems that the device ID handoff logic exists to paper over a deeper problem with the inspector proxy protocol (or its implementation in React Native): The React Native runtime should not routinely be creating new "device" connections without tearing down previous ones. In followup diffs, I'll explore changing this behaviour for Fusebox, based on the new test coverage. Reviewed By: robhogan Differential Revision: D51013056 fbshipit-source-id: e0c17678cc747366a3b75cef18ca2a722fc93acd
1 parent 9491ded commit 4c6bff0

File tree

5 files changed

+360
-4
lines changed

5 files changed

+360
-4
lines changed

packages/dev-middleware/src/__tests__/InspectorDeviceUtils.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import type {
2020
WrappedEvent,
2121
} from '../inspector-proxy/types';
2222

23+
import nullthrows from 'nullthrows';
2324
import WebSocket from 'ws';
2425

2526
export class DeviceAgent {
@@ -82,6 +83,11 @@ export class DeviceAgent {
8283
},
8384
});
8485
}
86+
87+
// $FlowIgnore[unsafe-getters-setters]
88+
get socket(): WebSocket {
89+
return nullthrows(this.#ws);
90+
}
8591
}
8692

8793
export class DeviceMock extends DeviceAgent {

packages/dev-middleware/src/__tests__/InspectorProtocolUtils.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,15 @@ export async function createAndConnectTarget(
126126
}>,
127127
signal: AbortSignal,
128128
page: PageFromDevice,
129+
deviceId: ?string = null,
129130
): Promise<{device: DeviceMock, debugger_: DebuggerMock}> {
130131
let device;
131132
let debugger_;
132133
try {
133134
device = await createDeviceMock(
134-
`${serverRef.serverBaseWsUrl}/inspector/device?device=device&name=foo&app=bar`,
135+
`${serverRef.serverBaseWsUrl}/inspector/device?device=${
136+
deviceId ?? 'device' + Date.now()
137+
}&name=foo&app=bar`,
135138
signal,
136139
);
137140
device.getPages.mockImplementation(() => [page]);

packages/dev-middleware/src/__tests__/InspectorProxyCdpTransport-test.js

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,10 @@
99
* @oncall react_native
1010
*/
1111

12-
import type {PageDescription} from '../inspector-proxy/types';
12+
import type {
13+
JsonPagesListResponse,
14+
PageDescription,
15+
} from '../inspector-proxy/types';
1316

1417
import {fetchJson} from './FetchUtils';
1518
import {createDebuggerMock} from './InspectorDebuggerUtils';
@@ -51,7 +54,7 @@ describe.each(['HTTP', 'HTTPS'])(
5154
},
5255
]);
5356

54-
let pageList: Array<PageDescription> = [];
57+
let pageList: JsonPagesListResponse = [];
5558
await until(async () => {
5659
pageList = (await fetchJson(
5760
`${serverRef.serverBaseUrl}/json`,

0 commit comments

Comments
 (0)