From 9ae812c72de91feee4e2263b231f269081d62afa Mon Sep 17 00:00:00 2001 From: Cedric van Putten Date: Fri, 4 Oct 2024 02:45:59 -0700 Subject: [PATCH] fix(dev-middleware): respond with status code `200` when launching RNDT (#46814) Summary: This fixes an issue where `POST /open-debugger?appId&device&target` does not return a proper status code, meaning that the request will never be answered and clients might hang until the request timeout is hit. ## Changelog: [GENERAL] [FIXED] - Respond with status code `200` when successfully launching RNDT Pull Request resolved: https://github.com/facebook/react-native/pull/46814 Test Plan: - `curl -v -X POST ""` - This should show a proper response for the request. before | after --- | --- ![image](https://github.com/user-attachments/assets/5b820acd-1168-4642-90ec-f2eeec0afc16) | ![image](https://github.com/user-attachments/assets/82bb2a6c-3c7b-483f-a4a1-ad00e5ca0178) Reviewed By: NickGerleman Differential Revision: D63837025 Pulled By: huntie fbshipit-source-id: ac72fc793e015f0eec498f4a35b4fb9e301c5b32 --- .../src/__tests__/FetchUtils.js | 2 +- .../__tests__/InspectorProxyHttpApi-test.js | 57 +++++++++++++++++++ .../src/middleware/openDebuggerMiddleware.js | 1 + 3 files changed, 59 insertions(+), 1 deletion(-) diff --git a/packages/dev-middleware/src/__tests__/FetchUtils.js b/packages/dev-middleware/src/__tests__/FetchUtils.js index 413ada1523e..43195383e80 100644 --- a/packages/dev-middleware/src/__tests__/FetchUtils.js +++ b/packages/dev-middleware/src/__tests__/FetchUtils.js @@ -21,7 +21,7 @@ declare var globalThis: $FlowFixMe; */ export async function fetchLocal( url: string, - options?: Parameters[1] & {dispatcher?: mixed}, + options?: Partial[1] & {dispatcher?: mixed}>, ): ReturnType { return await fetch(url, { ...options, diff --git a/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js b/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js index cb94b100bfb..52a53de214b 100644 --- a/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js +++ b/packages/dev-middleware/src/__tests__/InspectorProxyHttpApi-test.js @@ -18,6 +18,7 @@ import {fetchJson, fetchLocal} from './FetchUtils'; import {createDeviceMock} from './InspectorDeviceUtils'; import {withAbortSignalForEachTest} from './ResourceUtils'; import {withServerForEachTest} from './ServerUtils'; +import DefaultBrowserLauncher from '../utils/DefaultBrowserLauncher'; // Must be greater than or equal to PAGES_POLLING_INTERVAL in `InspectorProxy.js`. const PAGES_POLLING_DELAY = 1000; @@ -368,4 +369,60 @@ describe('inspector proxy HTTP API', () => { } }); }); + + describe('/open-debugger endpoint', () => { + it('opens requested device using appId, device, and target', async () => { + // Connect a device to use when opening the debugger + const device = await createDeviceMock( + `${serverRef.serverBaseWsUrl}/inspector/device?device=device1&name=foo&app=bar`, + autoCleanup.signal, + ); + device.getPages.mockImplementation(() => [ + { + app: 'bar-app', + id: 'page1', + title: 'bar-title', + vm: 'bar-vm', + capabilities: { + // Ensure the device target can be found when launching the debugger + nativePageReloads: true, + }, + }, + ]); + jest.advanceTimersByTime(PAGES_POLLING_DELAY); + + // Hook into `DefaultBrowserLauncher.launchDebuggerAppWindow` to ensure debugger was launched + const launchDebuggerSpy = jest + .spyOn(DefaultBrowserLauncher, 'launchDebuggerAppWindow') + .mockResolvedValueOnce(); + + try { + // Fetch the target information for the device + const pageListResponse = await fetchJson( + `${serverRef.serverBaseUrl}/json`, + ); + // Select the first target from the page list response + expect(pageListResponse.length).toBeGreaterThanOrEqual(1); + const firstPage = pageListResponse[0]; + + // Build the URL for the debugger + const openUrl = new URL('/open-debugger', serverRef.serverBaseUrl); + openUrl.searchParams.set('appId', firstPage.description); + openUrl.searchParams.set( + 'device', + firstPage.reactNative.logicalDeviceId, + ); + openUrl.searchParams.set('target', firstPage.id); + // Request to open the debugger for the first device + const response = await fetchLocal(openUrl.toString(), {method: 'POST'}); + + // Ensure the request was handled properly + expect(response.status).toBe(200); + // Ensure the debugger was launched + expect(launchDebuggerSpy).toHaveBeenCalledWith(expect.any(String)); + } finally { + device.close(); + } + }); + }); }); diff --git a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js index 69051ec09c3..97fcb9c984f 100644 --- a/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js +++ b/packages/dev-middleware/src/middleware/openDebuggerMiddleware.js @@ -132,6 +132,7 @@ export default function openDebuggerMiddleware({ {launchId, useFuseboxEntryPoint}, ), ); + res.writeHead(200); res.end(); break; case 'redirect':