Skip to content

Commit

Permalink
[DevTools] improve error handling in extension (#26068)
Browse files Browse the repository at this point in the history
## Summary

This is to fix some edge cases I recently observed when developing and
using the extension:
- When you reload the page, there's a chance that a port (most likely
the devtools one) is not properly unloaded. In this case, the React
DevTools will stop working unless you create a new tab.
- For unknown reasons, Chrome sometimes spins up two service worker
processes. In this case, an error will be thrown "duplicate ID when
registering content script" and sometimes interrupt the execution of the
rest of service worker.

This is an attempt to make the logic more robust 
- Automatically shutting down the double pipe if the message fails, and
allowing the runtime to rebuild the double pipe.
- Log the error message so Chrome believes we've handled it and will not
interrupt the execution.

This also seems to be helpful in fixing #25806.
  • Loading branch information
mondaychen authored Feb 7, 2023
1 parent 1445acf commit c12194f
Showing 1 changed file with 44 additions and 19 deletions.
63 changes: 44 additions & 19 deletions packages/react-devtools-extensions/src/background.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,31 @@ if (!IS_FIREFOX) {
// It's critical since it allows us to directly run scripts on the "main" world on the page
// "document_start" allows it to run before the page's scripts
// so the hook can be detected by react reconciler
chrome.scripting.registerContentScripts([
{
id: 'hook',
matches: ['<all_urls>'],
js: ['build/installHook.js'],
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
},
{
id: 'renderer',
matches: ['<all_urls>'],
js: ['build/renderer.js'],
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
chrome.scripting.registerContentScripts(
[
{
id: 'hook',
matches: ['<all_urls>'],
js: ['build/installHook.js'],
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
},
{
id: 'renderer',
matches: ['<all_urls>'],
js: ['build/renderer.js'],
runAt: 'document_start',
world: chrome.scripting.ExecutionWorld.MAIN,
},
],
function() {
// When the content scripts are already registered, an error will be thrown.
// It happens when the service worker process is incorrectly duplicated.
if (chrome.runtime.lastError) {
console.error(chrome.runtime.lastError);
}
},
]);
);
}

chrome.runtime.onConnect.addListener(function (port) {
Expand All @@ -51,7 +60,7 @@ chrome.runtime.onConnect.addListener(function (port) {
ports[tab][name] = port;

if (ports[tab].devtools && ports[tab]['content-script']) {
doublePipe(ports[tab].devtools, ports[tab]['content-script']);
doublePipe(ports[tab].devtools, ports[tab]['content-script'], tab);
}
});

Expand All @@ -70,20 +79,36 @@ function installProxy(tabId: number) {
}
}

function doublePipe(one, two) {
function doublePipe(one, two, tabId) {
one.onMessage.addListener(lOne);
function lOne(message) {
two.postMessage(message);
try {
two.postMessage(message);
} catch (e) {
if (__DEV__) {
console.log(`Broken pipe ${tabId}: `, e);
}
shutdown();
}
}
two.onMessage.addListener(lTwo);
function lTwo(message) {
one.postMessage(message);
try {
one.postMessage(message);
} catch (e) {
if (__DEV__) {
console.log(`Broken pipe ${tabId}: `, e);
}
shutdown();
}
}
function shutdown() {
one.onMessage.removeListener(lOne);
two.onMessage.removeListener(lTwo);
one.disconnect();
two.disconnect();
// clean up so that we can rebuild the double pipe if the page is reloaded
ports[tabId] = null;
}
one.onDisconnect.addListener(shutdown);
two.onDisconnect.addListener(shutdown);
Expand Down

0 comments on commit c12194f

Please sign in to comment.