From c12194f7485f298fadc1e51cfffb93e63d61ad96 Mon Sep 17 00:00:00 2001 From: Mengdi Chen Date: Tue, 7 Feb 2023 07:59:44 -0500 Subject: [PATCH] [DevTools] improve error handling in extension (#26068) ## 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. --- .../src/background.js | 63 +++++++++++++------ 1 file changed, 44 insertions(+), 19 deletions(-) diff --git a/packages/react-devtools-extensions/src/background.js b/packages/react-devtools-extensions/src/background.js index 70c2420b0df18..8ef96d2ec9b6f 100644 --- a/packages/react-devtools-extensions/src/background.js +++ b/packages/react-devtools-extensions/src/background.js @@ -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: [''], - js: ['build/installHook.js'], - runAt: 'document_start', - world: chrome.scripting.ExecutionWorld.MAIN, - }, - { - id: 'renderer', - matches: [''], - js: ['build/renderer.js'], - runAt: 'document_start', - world: chrome.scripting.ExecutionWorld.MAIN, + chrome.scripting.registerContentScripts( + [ + { + id: 'hook', + matches: [''], + js: ['build/installHook.js'], + runAt: 'document_start', + world: chrome.scripting.ExecutionWorld.MAIN, + }, + { + id: 'renderer', + matches: [''], + 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) { @@ -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); } }); @@ -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);