Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ensure port is connected #2533

Merged
merged 2 commits into from
Dec 24, 2023
Merged

ensure port is connected #2533

merged 2 commits into from
Dec 24, 2023

Conversation

patricklx
Copy link
Collaborator

@patricklx patricklx commented Dec 20, 2023

fixes #2532

looks like service workers can be shutown after idle time... with manifest V3.
but it can be stopped any time, as it is not guaranteed.
also we reconnect immediately so that we can receive updates when some start coming in from ember_debug site
https://developer.chrome.com/docs/extensions/develop/concepts/service-workers/lifecycle#idle-shutdown

tested manually by stopping the service worker in chrome://serviceworker-internals/?devtools

@patricklx patricklx marked this pull request as ready for review December 21, 2023 14:40
@patricklx patricklx closed this Dec 21, 2023
@patricklx patricklx reopened this Dec 21, 2023
@kdagnan
Copy link
Contributor

kdagnan commented Dec 21, 2023

Thank you! We will run this version today and report back

@kdagnan
Copy link
Contributor

kdagnan commented Dec 21, 2023

One freeze so far today. These error messages were all there yesterday but the disconnectedPort one is gone now:
image

@patricklx
Copy link
Collaborator Author

patricklx commented Dec 22, 2023

I wonder what that last error is. Are you using chrome or something via visual studio?
Edit: confirmed that this specific error is a chrome bug. Has been fixed recently:
https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/5067599.
happens when dragging the menu items , but does not impact ember-inspector

@patricklx patricklx marked this pull request as draft December 22, 2023 09:37
@patricklx
Copy link
Collaborator Author

@kdagnan I added some logging, can you test with this?

@patricklx patricklx marked this pull request as ready for review December 22, 2023 15:32
@patricklx
Copy link
Collaborator Author

i added the logging to https://github.com/patricklx/ember-inspector/tree/port-connect-logging

i think this already makes sense to merge, a similar issue was reported on discord

@patricklx
Copy link
Collaborator Author

@RobbieTheWagner

@kdagnan
Copy link
Contributor

kdagnan commented Dec 22, 2023

I think that the _ensureConnected change actually fixed the original issue, I just realized the only time I am having issues now are when stopped at a breakpoint in the browser. Inspector freezes up, but when breakpoints are all stepped through, it works again.

Installed 4.9.1 and it had the same behavior although I could've sworn it worked previously.

Thank you again for your help & quick fixes.

let chromePort = this._chromePort;
chromePort.postMessage({ appId: chrome.devtools.inspectedWindow.tabId });

chromePort.onMessage.addListener((...args) => {
this._messageReceived(...args);
});

chromePort.onDisconnect.addListener(() => {
this._isConnected = false;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need this this._isConnected boolean? If we're going to call this._connect onDisconnect wouldn't that be sufficient?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't. fixed

@RobbieTheWagner RobbieTheWagner merged commit 5d0b8ed into emberjs:main Dec 24, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inspector Becomes Unresponsive When Left Open for a Few Minutes
3 participants