Skip to content

[2135] Expose Dashboard client connection/disconnection state to Node-RED flows - #2208

Open
n-lark wants to merge 5 commits into
mainfrom
2135-client-presence
Open

[2135] Expose Dashboard client connection/disconnection state to Node-RED flows#2208
n-lark wants to merge 5 commits into
mainfrom
2135-client-presence

Conversation

@n-lark

@n-lark n-lark commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Description

See #2135 (comment)
Changelog here: FlowFuse/website#5736

Related Issue(s)

Resolves #2135

Checklist

  • I have read the contribution guidelines
  • Suitable unit/system level tests have been added and they pass
  • Documentation has been updated
    • Upgrade instructions
    • Configuration details
    • Concepts
  • Changes flowforge.yml?
    • Issue/PR raised on FlowFuse/helm to update ConfigMap Template
    • Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production
  • Link to Changelog Entry PR, or note why one is not needed.

Labels

  • Includes a DB migration? -> add the area:migration label

…connect/gone) to ui-control, keyed on a durable clientId instead of the ephemeral socket.id
@n-lark n-lark self-assigned this Sep 2, 2026

@Steve-Mcl Steve-Mcl left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Tested locally. Conditionally approved (I am on vacation after today)

Nit 1

Currently, it emits a payload string with client-connect / client-reconnect / client-gone

The first 2 are present tense and the last is past tense.
in my head, connect and reconnect means "now" however these messages occur after the event -so would client-connected / client-reconnected / client-gone not make more sense?

Nit 2

I could not get client-reconnect to fire unless i refreshed the dashboard (no amount of blocking the comms then recovery seemed to trigger it:

Image

Happy to trust that was intended or a windows quirk

@n-lark

n-lark commented Sep 4, 2026

Copy link
Copy Markdown
Contributor Author

Hey @Steve-Mcl ty for the review and testing!

Nit 1

Updated to client-connected / client-reconnected / client-gone

Nit 2

Yahhh so for reconnected, it fires when the same browser leaves aka socket closes and returns within the 20s grace window. Blocking comms doesn't cleanly close the socket, so you won't see it that way. Ways to see it:

  • Refresh the page
  • Close the tab, then reopen in the same browser within 20s

@n-lark
n-lark requested a review from Steve-Mcl September 4, 2026 14:04
}
```

The grace window means a brief blip or a page refresh does **not** fire `client-gone`; only a genuine departure does. Opening a second tab of a client that is already connected emits nothing (it is already present).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wondering if this is ideal behaviour.

One of the reasons to expose socketId at all is to be able to direct a message to just that dashboard.

I may open the same dashboard in two tabs, and want to treat them as separate 'sessions'. Each of those sessions would be identifiable via clientId/socketId pair - but only if I know about them.

If we don't emit the client connected events for a second connection, we lose the ability to track all of the open dashboards.

Could we instead emit the events with an additional sockets property that is an array of all active sockets we see this clientId on?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yahhh so I think you can accomplish that here by combining the socket level events with the client-presence events. The socket connect/lost fire per tab and already come through with the clientId, so you can reconcile from that end and get the data you need.

The client-connected/reconnected/gone events are meant to be client-level presence with the grace window, not per-tab. So if you wanted to completely manage both you could use both channels, totally open to adding a sockets array to the presence events if it's useful, but firing one of these on every socket connect/disconnect feels like it'd just duplicate the socket events.

@n-lark
n-lark requested a review from knolleary September 4, 2026 17:10
@cstns

cstns commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

I pulled this branch and spent some time testing the presence paths locally: built the UI, ran it in Node-RED with a small flow capturing the ui-control output, and drove it with a couple of separate browsers plus a raw socket.io client for the edge cases. Sharing what I found, roughly in order of how much I think it matters.

Targeting a message by clientId broadcasts it to every client. With "Accepts Client Data" active, I sent a notification with msg._client.clientId set to one browser's id and it showed up on both browsers. Sending the same notification with msg._client.socketId correctly reached only the intended one. So isValidConnection only honors socketId, and since the docs position clientId as the stable per-client key, a flow author will reasonably use it for targeting and silently leak data to everyone. Either wiring clientId into connection targeting or a very loud docs note feels necessary here.

Presence tracking breaks permanently once the dashboard is removed and re-added. I deployed a flow without any dashboard nodes while a client was connected, then restored the dashboard. The client reconnected fine but no client-connected or client-reconnected fired, and when it later left for good, client-gone never came (well past the grace window). Looks like the close path strips the socket disconnect listeners before the sockets are dropped, so the client store keeps a stale socket entry forever. The store probably needs to be drained or reset when the base node closes.

A ui-control node deployed while clients are already connected never learns about them, but does get their client-gone later. So the documented pattern of keeping per-client state (add on connected, remove on gone) drifts: you receive gone events for clients you never saw connect. Might be worth replaying the current presence state to newly registered listeners, or documenting the limitation.

The new client-* events also fire under "All Events", which is the default. A ui-control left on its default config starts receiving the new payloads after upgrading. Existing flows that count connections or have a catch-all branch on msg.payload will see new messages. Gating them to the explicit "Client Presence" option, or at least flagging it in the release notes, would avoid the surprise.

The presence events carry a bare _client. On the same node, connect includes socketIp (and whatever auth plugins add), while client-connected has just clientId and socketId. For the multi-tenancy use case that probably matters most, you can see that someone left but not who. Routing these through addConnectionCredentials where a socket is available might be worth considering.

A few smaller ones:

  • A pending grace timer keeps the Node process alive. A bare script that connects and disconnects one client takes the full 20 seconds to exit, since the timeout is neither unref'd nor cancelled anywhere.
  • clientId comes straight off the handshake query unvalidated. Sending the param twice produces an array in msg._client.clientId while the store coerces it to a comma-joined string for its key, so flow state and presence events can't match. A client with no clientId at all (e.g. a cached pre-upgrade bundle on a reconnected socket) ends up with the key present but undefined in _client. A single normalizing accessor for reading it would cover both.
  • The stability promise in the docs doesn't always hold: in a browser where storage writes fail (quota exceeded style), every reload generates a fresh clientId, so you get connected/gone churn from a single user. Probably just needs a docs caveat.
  • Unrelated to this PR but found while testing: when storage reads throw, the dashboard fails to load entirely, crashing in retrieveDefaultThemeFromCache. Happy to raise that separately.

The happy paths all worked well in my testing: stable id across reloads, client-reconnected within grace, correct multi-tab behavior, and client-gone timing. The store logic itself reads clean, these are all edges around it.

@cstns cstns left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

see the comment above

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.

Expose Dashboard client connection/disconnection state to Node-RED flows

4 participants