Skip to content

Send an event notifying DWDS when the debugger is detached #1595

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

Merged
merged 3 commits into from
May 4, 2022

Conversation

elliette
Copy link
Contributor

@elliette elliette commented May 3, 2022

Seems to resolve b/231336802

We can get in a weird state with the Dart Debug Extension. If a user starts debugging, and detaches the debugger by closing the banner added by Chrome, we close the SSE socket client.

However, from DWDS, there can be a substantial delay before the SSE socket stream receives the "done" event, and therefore closes the actual SSE connection (code).

If this done event is received AFTER the user has already starting debugging their application again, then we close their debug session while they are debugging.

It seems like this is resolved if we send an event to notify DWDS that the debugger has been detached, and call close in response to that event. I'm not sure why this stream event is received immediately, but the done event isn't.

@elliette elliette requested review from annagrin and grouma May 3, 2022 23:41
@grouma
Copy link
Member

grouma commented May 4, 2022

I'm not sure why this stream event is received immediately, but the done event isn't.

I believe this is due to the keepAlive mechanism in package:sse here: https://github.com/dart-lang/sse/blob/2df072848a6090d3ed67f30c69e86ec4d6b96cd6/lib/src/server/sse_handler.dart#L106

I think this is a suitable solution but we should update the comments.

@elliette
Copy link
Contributor Author

elliette commented May 4, 2022

I think this is a suitable solution but we should update the comments.

SG! Comments have been updated

@elliette elliette merged commit a0b9bae into dart-lang:master May 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants