[DevTools] improve error handling in extension #26068
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
React Core Team
tyao1
reviewed
Feb 2, 2023
| doublePipe(ports[tab].devtools, ports[tab]['content-script']); | ||
| doublePipe(ports[tab].devtools, ports[tab]['content-script'], tab); | ||
| // clean up so that we can rebuild the double pipe if the page is reloaded | ||
| ports[tab] = null; |
There was a problem hiding this comment.
Should this be moved to shutdown? On line 176, ports[id] is accessed. (although I don't know what it does)
There was a problem hiding this comment.
Good catch. Fixed.
| if (__DEV__) { | ||
| console.log(`Broken pipe ${tabId}: `, e); | ||
| } | ||
| shutdown(); |
There was a problem hiding this comment.
Would the postMessage fail because of a JS exception? I'm not sure if it will happen. I think if we shutdown here not during a page load, the devtools would stop working?
There was a problem hiding this comment.
It's not a JS exception. It's a service worker exception when postMessage is fired on a tab that's offloaded. The error message looks like this:
It's weird that the listener was not removed since we already have one.onDisconnect.addListener(shutdown). Maybe Chrome doesn't treat reload as disconnect anymore?
mondaychen
changed the title
tyao1
approved these changes
Feb 7, 2023
github-actions bot
pushed a commit
that referenced
this pull request
Feb 7, 2023
## 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. DiffTrain build for [c12194f](c12194f) [View git log for this commit](https://github.com/facebook/react/commits/c12194f7485f298fadc1e51cfffb93e63d61ad96)
eps1lon
added a commit
that referenced
this pull request
Feb 7, 2023
## Summary Prettier was bumped recently. So any branch not including that bump, might bring in outdated formatting (e.g. #26068) ## How did you test this change? - [x] `yarn prettier-all`
github-actions bot
pushed a commit
that referenced
this pull request
Feb 7, 2023
## Summary Prettier was bumped recently. So any branch not including that bump, might bring in outdated formatting (e.g. #26068) ## How did you test this change? - [x] `yarn prettier-all` DiffTrain build for [13f4ccf](13f4ccf) [View git log for this commit](https://github.com/facebook/react/commits/13f4ccfdba06b599a5db7c5a192024cbfa365edc)
jerrydev0927
added a commit
to jerrydev0927/react
that referenced
this pull request
Jan 5, 2024
## Summary Prettier was bumped recently. So any branch not including that bump, might bring in outdated formatting (e.g. facebook/react#26068) ## How did you test this change? - [x] `yarn prettier-all` DiffTrain build for [13f4ccfdba06b599a5db7c5a192024cbfa365edc](facebook/react@13f4ccf) [View git log for this commit](https://github.com/facebook/react/commits/13f4ccfdba06b599a5db7c5a192024cbfa365edc)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This is to fix some edge cases I recently observed when developing and using the extension:
This is an attempt to make the logic more robust
This also seems to be helpful in fixing #25806.