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

DevTools no longer opens in browser if a previous window was closed #3966

Closed
DanTup opened this issue May 17, 2022 · 7 comments · Fixed by flutter/devtools#4355
Closed

DevTools no longer opens in browser if a previous window was closed #3966

DanTup opened this issue May 17, 2022 · 7 comments · Fixed by flutter/devtools#4355
Labels
in devtools Relates to integration with Dart/Flutter DevTools is bug
Milestone

Comments

@DanTup
Copy link
Member

DanTup commented May 17, 2022

  • run a debug session
  • run the Dart: Open DevTools command
  • select Open DevTools in Web Browser
  • close the browser window
  • try to open again in the browser
  • (nothing happens)

I think the DevTools server is not correctly handling the browser being closed, so it tries to reuse a connected client that doesn't exist (rather than launch a new one).

@DanTup DanTup added is bug in devtools Relates to integration with Dart/Flutter DevTools labels May 17, 2022
@DanTup DanTup added this to the v3.42.0 milestone May 17, 2022
@DanTup DanTup changed the title DevTools now longer opens in browser if a previous window was closed DevTools no longer opens in browser if a previous window was closed May 17, 2022
@DanTup
Copy link
Member Author

DanTup commented May 17, 2022

Turns out that this is working, but there is a 30 second delay between closing the browser and DevTools recording that client as gone. This is because the SSE connection has a 30 second keepalive so after closing the browser as far as DevTools is concerned, that client is still open for another 30 seconds and available for reuse:

https://github.com/dart-lang/sdk/blob/e995cb5f7cd67d39c1ee4bdbe95c8241db36725f/pkg/dds/lib/src/constants.dart#L17-L19

// Give connections time to reestablish before considering them closed.
// Required to reestablish connections killed by UberProxy.
const sseKeepAlive = Duration(seconds: 30);

@jacob314 @kenzieschmoll I'm not sure there's a good solution here.. Reducing the SSE timeout may affect UberProxy users (based on the comment above).

One option could be to just disable the reuse of windows from VS Code. This means if the user disables embedded DevTools and clicks the Inspector icon multiple times, we would launch a new window with the inspector each time (whereas today we still just show a browser notification saying "hey, you already have this one open here").

@DanTup DanTup added the awaiting info Requires more information from the customer to progress label May 17, 2022
@DanTup DanTup modified the milestones: v3.42.0, v3.44.0 May 25, 2022
@github-actions
Copy link

This issue has been marked stale because it is tagged awaiting-info for 20 days with no activity. Remove the stale label or comment to prevent the issue being closed in 10 days.

@github-actions github-actions bot added the stale Will be closed soon if no response. label Jun 15, 2022
@DanTup DanTup modified the milestones: v3.44.0, v3.46.0 Jun 22, 2022
@github-actions github-actions bot closed this as completed Jul 3, 2022
@DanTup DanTup reopened this Jul 4, 2022
@DanTup DanTup removed the stale Will be closed soon if no response. label Jul 4, 2022
@github-actions
Copy link

This issue has been marked stale because it is tagged awaiting-info for 20 days with no activity. Remove the stale label or comment to prevent the issue being closed in 10 days.

@github-actions github-actions bot added the stale Will be closed soon if no response. label Jul 25, 2022
@DanTup DanTup removed awaiting info Requires more information from the customer to progress stale Will be closed soon if no response. labels Jul 25, 2022
@DanTup DanTup modified the milestones: v3.46.0, v3.48.0 Jul 25, 2022
@jacob314
Copy link

I would use a ping API first to verify the browser is still alive.

@DanTup
Copy link
Member Author

DanTup commented Aug 22, 2022

Oops, didn't realise committing to another repo would close this. The change that landed in DevTools is only the first part, this requires some additional work in the server in the SDK.

@DanTup DanTup reopened this Aug 22, 2022
@DanTup DanTup modified the milestones: v3.48.0, v3.50.0 Aug 23, 2022
@DanTup
Copy link
Member Author

DanTup commented Sep 5, 2022

The other change is open at https://dart-review.googlesource.com/c/sdk/+/257581/. I've confirmed locally (using this method of running in VS Code) that it now re-uses the existing window if it's there, but opens a new one if not (although, it required a Ctrl+Refresh to bust the cache so the client responded to the ping).

copybara-service bot pushed a commit to dart-lang/sdk that referenced this issue Sep 7, 2022
It's possible the browser has been closed but is in the SSE timeout period and therefore looks active. Ping it to see if it's actually responsive before deciding whether to reuse it or launch a new window.

Fixes Dart-Code/Dart-Code#3966.

Change-Id: I2fdcba036b8b63f7ab974e8fef5dd565c2917b64
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/257581
Reviewed-by: Kenzie Davisson <kenzieschmoll@google.com>
Commit-Queue: Ben Konyi <bkonyi@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
@DanTup
Copy link
Member Author

DanTup commented Sep 7, 2022

Fixed by dart-lang/sdk@70e8dc6. Will ship as part of an SDK, not Dart-Code.

@DanTup DanTup closed this as completed Sep 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in devtools Relates to integration with Dart/Flutter DevTools is bug
Projects
None yet
2 participants