-
Notifications
You must be signed in to change notification settings - Fork 366
DTDManager handles the subscription to the service stream #9635
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
base: master
Are you sure you want to change the base?
Conversation
| if (_connection.value case final connection?) { | ||
| await connection.close(); | ||
| } | ||
| _connection.value = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I noticed that we were setting the connection to null without closing it first
| _connection.value = connection; | ||
| // Close the previous connection. | ||
| if (previousConnection != null) { | ||
| await previousConnection.close(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before this change, we were closing the connection during the disconnect implementation. I moved it here to the connect implementation instead so that we wait for the new connection before closing the previous connection (this prevents us from dropping any service extension registration events we receive while reconnecting).
Fixes #9632
DTDManagernow subscribes to theServicestream, and broadcastsServiceRegisteredandServiceUnregisteredevents to any subscribers.Currently the
EditorClientis the only subscriber, but our AI Assistant will also need to subscribe to those events.FYI @DanTup I've changed the DTD reconnection logic a little bit. I tested the Flutter panel and Property Editor in VS Code, but you might want to verify as well.