mirror of
https://github.com/facebook/react-native.git
synced 2024-11-21 22:10:14 +00:00
dev-middleware: Fix reliance on adb reverse
when loading sources on Android (#47652)
Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47652 ## Background When the `nativeSourceCodeFetching` capability is disabled, `inspector-proxy` rewrites URLs exchanged over CDP between device and frontend so that URLs are addressable from CDT - in particular, when using an Android emulator `10.0.2.2` (host's address from within the emulator) is rewritten to and from `localhost` (the equivalent address reachable from the host). Previously - before we implemented `Network.loadNetworkResource`, or on old frontends that don't attempt to use that method - this worked reasonably well. A `fetch` from CDT to Metro would succeed on the rewritten URL. ## Problem Since we implemented `Network.loadNetworkResource`, but disabled the `nativeSourceCodeFetching` capability, source fetching is broken under Android emulators. We're rewriting URLs to be frontend-relative, but then attempting to fetch them through the device, because as far as CDT is aware, `Network.loadNetworkResource` should still be tried first. When `Network.loadNetworkResource` responds with a CDP *error*, CDT falls back to a local fetch (which would work), but when it responds with a CDP *result* of `success: false`, there is no fallback. ## Fix This diff adds an interception guarded behind `nativeSourceCodeFetching == false`, which rejects any calls to `Network.loadNetworkResource` with a CDP error. This restores the previous behaviour from before `Network.loadNetworkResource` was implemented at all. NOTE: An alternative approach would be to rewrite URLs back to device-relative for `Network.loadNetworkResource`, but IMO it's more correct for the frontend to respect that the device is asserting that it doesn't have that capability, and not to try to use it. Changelog: [Android][Fixed] RN DevTools: Fix source loading when using an Android emulator connecting to a dev server on the host. Reviewed By: huntie Differential Revision: D66074731 fbshipit-source-id: f2050c014cd5cfa546bff5e9d0412413a5daff35
This commit is contained in:
parent
8507204b53
commit
ca9c56329f
@ -295,6 +295,42 @@ describe.each(['HTTP', 'HTTPS'])(
|
||||
debugger_.close();
|
||||
}
|
||||
});
|
||||
|
||||
describe('Network.loadNetworkResource', () => {
|
||||
test('should respond with an error without forwarding to the client', async () => {
|
||||
const {device, debugger_} = await createAndConnectTarget(
|
||||
serverRef,
|
||||
autoCleanup.signal,
|
||||
{
|
||||
app: 'bar-app',
|
||||
id: 'page1',
|
||||
title: 'bar-title',
|
||||
vm: 'bar-vm',
|
||||
},
|
||||
);
|
||||
try {
|
||||
const response = await debugger_.sendAndGetResponse({
|
||||
id: 1,
|
||||
method: 'Network.loadNetworkResource',
|
||||
params: {
|
||||
url: 'http://example.com',
|
||||
},
|
||||
});
|
||||
expect(response.result).toEqual(
|
||||
expect.objectContaining({
|
||||
error: {
|
||||
code: -32601,
|
||||
message:
|
||||
'[inspector-proxy]: Page lacks nativeSourceCodeFetching capability.',
|
||||
},
|
||||
}),
|
||||
);
|
||||
} finally {
|
||||
device.close();
|
||||
debugger_.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
},
|
||||
);
|
||||
|
||||
@ -581,6 +617,33 @@ describe.each(['HTTP', 'HTTPS'])(
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('Network.loadNetworkResource', () => {
|
||||
test('should forward event directly to client (does not rewrite url host)', async () => {
|
||||
const {device, debugger_} = await createAndConnectTarget(
|
||||
serverRef,
|
||||
autoCleanup.signal,
|
||||
pageDescription,
|
||||
);
|
||||
try {
|
||||
const message = {
|
||||
id: 1,
|
||||
method: 'Network.loadNetworkResource',
|
||||
params: {
|
||||
url: `${protocol.toLowerCase()}://10.0.2.2:${serverRef.port}`,
|
||||
},
|
||||
};
|
||||
await sendFromDebuggerToTarget(debugger_, device, 'page1', message);
|
||||
expect(device.wrappedEventParsed).toBeCalledWith({
|
||||
pageId: 'page1',
|
||||
wrappedEvent: message,
|
||||
});
|
||||
} finally {
|
||||
device.close();
|
||||
debugger_.close();
|
||||
}
|
||||
});
|
||||
});
|
||||
});
|
||||
},
|
||||
);
|
||||
|
@ -799,6 +799,31 @@ export default class Device {
|
||||
// Sends response to debugger via side-effect
|
||||
this.#processDebuggerGetScriptSource(req, socket);
|
||||
return null;
|
||||
case 'Network.loadNetworkResource':
|
||||
// If we're rewriting URLs (to frontend-relative), we don't want to
|
||||
// pass these URLs to the device, since it may try to fetch, return a
|
||||
// CDP *result* (not error) with a network failure, and CDT
|
||||
// will *not* then fall back to fetching locally.
|
||||
//
|
||||
// Instead, take the absence of a nativeSourceCodeFetching
|
||||
// capability as a signal to never pass a loadNetworkResource request
|
||||
// to the device. By returning a CDP error, the frontend should fetch.
|
||||
const result = {
|
||||
error: {
|
||||
code: -32601, // Method not found
|
||||
message:
|
||||
'[inspector-proxy]: Page lacks nativeSourceCodeFetching capability.',
|
||||
},
|
||||
};
|
||||
const response = {id: req.id, result};
|
||||
socket.send(JSON.stringify(response));
|
||||
const pageId = this.#debuggerConnection?.pageId ?? null;
|
||||
this.#deviceEventReporter?.logResponse(response, 'proxy', {
|
||||
pageId,
|
||||
frontendUserAgent: this.#debuggerConnection?.userAgent ?? null,
|
||||
prefersFuseboxFrontend: this.#isPageFuseboxFrontend(pageId),
|
||||
});
|
||||
return null;
|
||||
default:
|
||||
return req;
|
||||
}
|
||||
|
@ -44,6 +44,7 @@ export type CDPClientMessage =
|
||||
| CDPRequest<'Debugger.getScriptSource'>
|
||||
| CDPRequest<'Debugger.scriptParsed'>
|
||||
| CDPRequest<'Debugger.setBreakpointByUrl'>
|
||||
| CDPRequest<'Network.loadNetworkResource'>
|
||||
| CDPRequest<>;
|
||||
|
||||
export type CDPServerMessage =
|
||||
|
Loading…
Reference in New Issue
Block a user