Skip to content

Commit 4a0236a

Browse files
apiadclaude
andcommitted
fix(client): per-tab client_id via module cache, not sessionStorage
Fixes gap 7.9: when two browser tabs share sessionStorage (Chrome's "Duplicate Tab" copies it), both ended up with the same VIOLETEAR_ID, causing the second connect to overwrite the first in the server's active_connections — broadcasts then reached only the most-recently- connected tab, manifesting as "messages only arrive on one or the other" in real multi-user testing of example 05. Replaces the sessionStorage-backed cache with a module-level _CLIENT_ID inside the bundle's Pyodide instance. Each tab has its own Pyodide, so the cache is per-tab; tab-duplicate spawns a fresh Pyodide → fresh UUID. As a side effect, also fixes gap 7.8: the read-back no longer goes through the Thing wrapper, so get_client_id reliably returns a bare str on every call. Example 05 drops its str(...) workarounds. Tradeoff documented in issues/7.9: a reloaded tab now gets a new id rather than reclaiming the prior one. Acceptable for the chat example; stateful apps should layer their own stable-id mechanism. Verified by reproducing the collision with two Playwright contexts pre-seeded with the same VIOLETEAR_ID — pre-fix: 0 messages reached the first tab; post-fix: all 6 messages reach both. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 80fc920 commit 4a0236a

3 files changed

Lines changed: 32 additions & 20 deletions

File tree

examples/05_realtime.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -159,22 +159,17 @@ async def on_socket_connect():
159159
# WS is now live — ask the server to push our initial state. The server
160160
# responds via receive_history.invoke(client_id=...) so only this client
161161
# gets it (proves the targeted-invoke wire path).
162-
#
163-
# `str(get_client_id())` because on second call `get_client_id()` returns
164-
# a `Thing` proxy (read through session storage) rather than a bare str,
165-
# and `_call_realtime` json.dumps the kwargs — Thing isn't serializable
166-
# (see issues/7.8). str() coerces via Thing.__repr__ → the wrapped value.
167162
from violetear.client import get_client_id
168163

169-
await request_history(client_id=str(get_client_id()))
164+
await request_history(client_id=get_client_id())
170165

171166

172167
@app.client.on("ready")
173168
async def on_ready():
174169
# Surface the default name in the rename field so the user can edit it.
175170
from violetear.client import get_client_id
176171

177-
my_id = str(get_client_id())
172+
my_id = get_client_id()
178173
name_input = DOM.find("name-input")
179174
name_input.value = f"anon-{my_id[:6]}"
180175

@@ -187,7 +182,7 @@ async def on_send_click(event: Event):
187182
text = str(text_el.value or "")
188183
if not text.strip():
189184
return
190-
await post_message(client_id=str(get_client_id()), text=text)
185+
await post_message(client_id=get_client_id(), text=text)
191186
text_el.value = ""
192187

193188

@@ -199,7 +194,7 @@ async def on_rename_click(event: Event):
199194
new_name = str(name_el.value or "")
200195
if not new_name.strip():
201196
return
202-
await set_name(client_id=str(get_client_id()), new_name=new_name)
197+
await set_name(client_id=get_client_id(), new_name=new_name)
203198

204199

205200
# ---------------------------------------------------------------------------

issues/7-framework-gaps-from-canonical-examples.md

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -89,17 +89,25 @@ Running log of small framework limitations and ergonomic friction discovered whi
8989

9090
**Impact.** Reference examples can't use `for=` / `class=` via kwargs without producing invalid HTML. Subtle because the form still submits (browsers ignore unknown attrs), so the bug is silent until someone inspects the rendered HTML.
9191

92-
## 7.8 — `get_client_id()` returns a `Thing` proxy on subsequent calls
92+
## 7.8 — `get_client_id()` returned a `Thing` proxy on subsequent calls (FIXED)
9393

9494
**Tier(s):** 05 (caught by Playwright e2e — silent in unit tests)
9595

96-
**Symptom.** `violetear.client.get_client_id()` is documented as returning a `str`. On first call (no session storage entry) it does — it generates a UUID, writes it via `session["VIOLETEAR_ID"] = client_id`, and returns the bare `str`. On every subsequent call it goes through `session.get("VIOLETEAR_ID")`, which returns a `storage.Thing` wrapper (the auto-persist proxy used for nested object mutation tracking) — not a `str`. Calling `_call_realtime(..., kwargs={"client_id": <Thing>})` then `json.dumps` the kwargs and blows up: `TypeError: Object of type Thing is not JSON serializable`. The error surfaces only in Pyodide at runtime; unit tests using `TestClient.websocket_connect` pass the client_id directly as a string and never hit this path.
96+
**Symptom.** `violetear.client.get_client_id()` promised `-> str` but only delivered on the first call. It seeded `session["VIOLETEAR_ID"]` with a fresh UUID string, then on subsequent calls read it back via `session.get(...)`which goes through `StorageAPI.__getitem__` and returns a `storage.Thing` wrapper (not a `str`). Calling `_call_realtime(..., kwargs={"client_id": <Thing>})` would then fail at `json.dumps` with `TypeError: Object of type Thing is not JSON serializable`. Silent in unit tests because `TestClient.websocket_connect` passes a string `client_id` directly; only the real Pyodide path hit the read-back.
9797

98-
**Workaround applied.** Coerce explicitly at every call site in 05_realtime: `client_id=str(get_client_id())`. `Thing.__repr__` returns `f"{self._data}"`, so `str(Thing("abc"))` returns `"abc"`. Verbose, but works today.
98+
**Fix applied.** Rewrote `get_client_id` to use a module-level `_CLIENT_ID` cache instead of sessionStorage round-tripping. Side benefits documented under gap 7.9.
9999

100-
**Where to fix in the framework.** `violetear/client.py:get_client_id` — make the function always return a plain `str`. Either unwrap before returning (`if hasattr(client_id, "unwrap"): client_id = client_id.unwrap()`) or read through a path that bypasses `Thing` wrapping for primitive values. The latter is structurally cleaner — `StorageAPI.__getitem__` wraps everything in `Thing` even when the underlying value is a primitive string/number, which makes sense for dict/list mutation tracking but is overkill for primitives. A `StorageAPI.get_raw(key)` that skips the wrap would let internal helpers like `get_client_id` avoid the proxy entirely.
100+
## 7.9 — `client_id` collision when two tabs share sessionStorage
101101

102-
**Larger lesson.** Functions that promise a primitive return type but actually return a smart-wrapper on some code paths are a bug magnet — they pass through arithmetic, comparisons, and string formatting fine but explode at the first json/pickle boundary. Type-annotated `-> str` is a check the framework's own helpers should satisfy strictly.
102+
**Tier(s):** 05 (reported by Alex during real-browser multi-user testing — silent in single-client and isolated-context tests)
103+
104+
**Symptom.** Two browser tabs that share `sessionStorage` (this happens whenever Chrome's "Duplicate Tab" feature is used — the duplicated tab inherits the original's sessionStorage) end up with the **same** `VIOLETEAR_ID` value. Both tabs connect to `/_violetear/ws?client_id=<same>`. The server's `SocketManager.connect` does `self.active_connections[client_id] = websocket` — the second tab's connect *overwrites* the first. From then on, broadcasts iterate `active_connections.items()` and find only one entry per `client_id`; the first tab's WS is still open on the wire but the server has lost its reference. User-visible effect: messages reach only the most-recently-connected tab. Reproduces deterministically by pre-seeding two Playwright contexts with the same `VIOLETEAR_ID` (commit 80fc920 + manual debug).
105+
106+
**Fix applied (client side).** `get_client_id` no longer persists across page loads. A module-level `_CLIENT_ID = None` is initialized at bundle exec; the first call sets it to a fresh `uuid4()`; subsequent calls return the cached value. Each browser tab runs its own Pyodide instance, so the cache is per-tab — and duplicating a tab gives the new one a fresh Pyodide → fresh `_CLIENT_ID`. Also incidentally fixes gap 7.8 (no more `Thing` round-trip).
107+
108+
**Tradeoff.** A tab that reloads now gets a *new* `client_id` rather than reclaiming its previous one. For the chat example that's fine (no per-client persistent state). Apps that need a stable id across reloads should layer their own (e.g. a server-issued cookie, or an explicit `localStorage` write that the user opts into per-app).
109+
110+
**Server-side residual gap.** Even with the client fix, the server still trusts a client-supplied `client_id` query param and would overwrite an existing entry if two clients ever sent the same one. A stricter server would reject the duplicate (HTTP 409 / WS close), or multiplex (`active_connections[client_id] = list[WebSocket]`) and broadcast to all sockets for an id. For v1 the client-side fix is enough; revisit when adding auth or when the realtime pattern grows beyond fire-and-forget.
103111

104112
---
105113

violetear/client.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -213,12 +213,21 @@ async def _dispatch_client_event(event: str, *args):
213213
# --- NETWORKING & RPC ---
214214

215215

216-
def get_client_id():
217-
client_id = session.get("VIOLETEAR_ID")
218-
if client_id is None:
219-
client_id = str(uuid.uuid4())
220-
session["VIOLETEAR_ID"] = client_id
221-
return client_id
216+
# Module-level cache. Each browser tab gets its own Pyodide instance, so this
217+
# is per-tab — and we deliberately do NOT persist it in sessionStorage. Chrome
218+
# copies sessionStorage on "Duplicate Tab", which would let two tabs share the
219+
# same client_id and collide in the server's `active_connections` dict (the
220+
# second connect overwrites the first; broadcasts then reach only one tab).
221+
# A fresh UUID per page load also sidesteps the `Thing`-proxy round-trip that
222+
# made the storage-backed version return a non-str on subsequent calls.
223+
_CLIENT_ID: str | None = None
224+
225+
226+
def get_client_id() -> str:
227+
global _CLIENT_ID
228+
if _CLIENT_ID is None:
229+
_CLIENT_ID = str(uuid.uuid4())
230+
return _CLIENT_ID
222231

223232

224233
def get_socket_url():

0 commit comments

Comments
 (0)