frontend: add Zenoh singleton#3820
Merged
patrickelectric merged 2 commits intobluerobotics:masterfrom Mar 10, 2026
Merged
Conversation
Reviewer's GuideIntroduce a shared Zenoh session manager singleton and refactor several components to obtain sessions through it instead of creating and closing their own websocket connections, simplifying connection logic and avoiding duplicate sessions. Sequence diagram for shared Zenoh session acquisition via singletonsequenceDiagram
participant CloudTrayMenu
participant ZenohNetwork
participant ZenohManager
participant ZenohSession
CloudTrayMenu->>ZenohManager: getSession()
alt sessionPromise is null
ZenohManager->>ZenohManager: getWebsocketUrl()
ZenohManager->>ZenohManager: create Config(url)
ZenohManager->>ZenohSession: Session.open(config)
ZenohSession-->>ZenohManager: Session
ZenohManager->>ZenohManager: store Promise_Session_
else sessionPromise exists
ZenohManager->>ZenohManager: reuse existing Promise_Session_
end
ZenohManager-->>CloudTrayMenu: Promise_Session_
ZenohNetwork->>ZenohManager: getSession()
ZenohManager-->>ZenohNetwork: same Promise_Session_
CloudTrayMenu->>CloudTrayMenu: await getSession()
CloudTrayMenu->>CloudTrayMenu: use session for file sync
ZenohNetwork->>ZenohNetwork: await getSession()
ZenohNetwork->>ZenohNetwork: use session for topology discovery
Class diagram for ZenohManager singleton and consumersclassDiagram
class ZenohManager {
-instance ZenohManager
-sessionPromise Promise_Session_
-ZenohManager()
+getInstance() ZenohManager
+getWebsocketUrl() string
+getSession() Promise_Session_
}
class CloudTrayMenu {
-file_sync_session Session
+setupFileSync() Promise_boolean_
+disconnectCloud() Promise_void_
}
class ZenohNetwork {
-session Session
+setupZenoh() Promise_void_
+refreshNetwork() Promise_void_
+disconnectZenoh() Promise_void_
}
class ConsoleLogger {
-session Session
+init() Promise_void_
+shutdown() Promise_void_
}
class ZenohInspector {
-session Session
-subscriber Subscriber
-liveliness_subscriber Subscriber
+setupZenoh() Promise_void_
+disconnectZenoh() Promise_void_
}
ZenohManager <.. CloudTrayMenu : uses_getSession
ZenohManager <.. ZenohNetwork : uses_getSession
ZenohManager <.. ConsoleLogger : uses_getSession
ZenohManager <.. ZenohInspector : uses_getSession
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- The ZenohManager never closes the underlying Session and doesn’t expose any teardown/reset, so components that previously closed their sessions now leak a long‑lived connection; consider adding a
close()/reset()API and using it from callers that explicitly disconnect. - If
Session.openrejects,sessionPromiseremains a rejected promise and all subsequentgetSession()calls will keep failing; it would be safer to catch failures and clearsessionPromiseso a later retry can succeed. ZenohManager.getWebsocketUrl()directly accesseswindow, which can break SSR/tests; consider guarding for non‑browser environments or injecting the URL from the outside when needed.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The ZenohManager never closes the underlying Session and doesn’t expose any teardown/reset, so components that previously closed their sessions now leak a long‑lived connection; consider adding a `close()`/`reset()` API and using it from callers that explicitly disconnect.
- If `Session.open` rejects, `sessionPromise` remains a rejected promise and all subsequent `getSession()` calls will keep failing; it would be safer to catch failures and clear `sessionPromise` so a later retry can succeed.
- `ZenohManager.getWebsocketUrl()` directly accesses `window`, which can break SSR/tests; consider guarding for non‑browser environments or injecting the URL from the outside when needed.
## Individual Comments
### Comment 1
<location path="core/frontend/src/libs/zenoh/index.ts" line_range="6-15" />
<code_context>
+class ZenohManager {
+ private static instance: ZenohManager
+
+ private sessionPromise: Promise<Session> | null = null
+
+ private constructor() {}
+
+ public static getInstance(): ZenohManager {
+ if (!ZenohManager.instance) {
+ ZenohManager.instance = new ZenohManager()
+ }
+ return ZenohManager.instance
+ }
+
+ public static getWebsocketUrl(): string {
+ const protocol = window.location.protocol === 'https:' ? 'wss' : 'ws'
+ return `${protocol}://${window.location.host}/zenoh-api/`
+ }
+
+ public getSession(): Promise<Session> {
+ if (!this.sessionPromise) {
+ const url = ZenohManager.getWebsocketUrl()
+ const config = new Config(url)
+ this.sessionPromise = Session.open(config)
+ }
+ return this.sessionPromise
+ }
+}
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle failed Session.open calls so a rejected promise isn't cached forever
Because `sessionPromise` is never cleared, a rejected `Session.open(config)` will be cached and every future `getSession()` call will keep returning the same rejection until page reload. Wrap `Session.open` so that on rejection you reset `this.sessionPromise` (and optionally retry) before rethrowing:
```ts
if (!this.sessionPromise) {
const url = ZenohManager.getWebsocketUrl()
const config = new Config(url)
this.sessionPromise = Session.open(config)
.catch(err => {
this.sessionPromise = null
throw err
})
}
```
This preserves de-duplication while avoiding a permanent failure state.
</issue_to_address>
### Comment 2
<location path="core/frontend/src/libs/zenoh/index.ts" line_range="22-28" />
<code_context>
+ return `${protocol}://${window.location.host}/zenoh-api/`
+ }
+
+ public getSession(): Promise<Session> {
+ if (!this.sessionPromise) {
+ const url = ZenohManager.getWebsocketUrl()
+ const config = new Config(url)
+ this.sessionPromise = Session.open(config)
+ }
+ return this.sessionPromise
+ }
+}
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider exposing a close/reset mechanism for the shared Zenoh session
Call sites that used to close their own `Session` (`CloudTrayMenu`, `ZenohNetwork`, `ZenohInspector`, `ConsoleLogger`) now just drop references, so the shared session may stay open for the lifetime of the app. This removes the ability to explicitly close the websocket on teardown or to recreate a fresh session after long-lived connections degrade or the backend restarts.
Consider adding a `ZenohManager.close()` (or similar) that closes the underlying `Session` (if it exists) and resets `sessionPromise` to `null`, so components can explicitly trigger shutdown or reconnection without reintroducing per-component setup logic.
Suggested implementation:
```typescript
public getSession(): Promise<Session> {
if (!this.sessionPromise) {
const url = ZenohManager.getWebsocketUrl()
const config = new Config(url)
this.sessionPromise = Session.open(config)
}
return this.sessionPromise
}
public async close(): Promise<void> {
if (this.sessionPromise) {
try {
const session = await this.sessionPromise
await session.close()
} finally {
this.sessionPromise = null
}
}
}
public static async close(): Promise<void> {
if (!ZenohManager.instance) {
return
}
await ZenohManager.instance.close()
}
public getSession(): Promise<Session> {
if (!this.sessionPromise) {
```
I’ve assumed that:
1. `this.sessionPromise` is declared as `Promise<Session> | null`. If it is currently `Promise<Session>` only, update its type to be nullable.
2. `Session.close()` returns a `Promise<void>`. If it is synchronous, you can remove `await` and the `async` keywords where appropriate.
3. Call sites that need to explicitly reset the shared session should now call `await ZenohManager.close()` when tearing down or when forcing a reconnect.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
261b354 to
c7d3aa9
Compare
patrickelectric
approved these changes
Mar 10, 2026
This file contains hidden or 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
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 by Sourcery
Introduce a shared Zenoh session manager and update frontend components to use it instead of creating and closing their own sessions.
New Features:
Enhancements: