Device detection: confirm before disconnecting another device
Asked for directly: "same account, same device already in the party"
should never need a confirmation, "same account, a genuinely different
device" should ask before disconnecting it instead of silently
overwriting. There was no concept of "device" at all before this -
party.members is keyed by userId only.
- client/src/deviceId.ts (new): a UUID kept in localStorage (not
sessionStorage) - that's what makes "device" mean "browser", not
"tab". Falls back to an in-memory id if storage is unavailable.
- server/src/types.ts, client/src/api/party.ts: "join" now carries
{partyId, deviceId, force?} instead of just {partyId}; new
PartyMember.deviceId and "deviceConflict" server->client event (sent
only to the joining socket, never broadcast).
- server/src/realtime/SocketGateway.ts: the "join" handler now tells
three cases apart - no existing connection or the same deviceId
reconnecting (attach silently, booting any stale connection for that
same device first), a different deviceId with no force (emit
deviceConflict and stop - the socket stays connected but inert), or
force:true (the confirmed takeover - boot the old connection, take
the slot).
- server/src/services/PartyService.ts: join() is now idempotent -
returns the existing member instead of always building a fresh one.
This was itself a real bug independent of the guard below: the REST
"Rejoindre" route calls this every time regardless of membership, so
a re-click/reload/retry used to silently reset connection/isPaused/
deviceId.
- server/src/realtime/SocketGateway.ts: "leave"/"disconnect" now also
check party.members.get(userId)?.connection === socket before acting
- generalizes the earlier stale-connection crash fix to the new
reality where a member can have more than one connection attempt in
flight (the superseded one's own eventual "disconnect" must not be
read as a real leave).
- client/src/views/player/Player.ts, client/src/musicBar/MusicBar.ts:
onDeviceConflict wired to a plain confirm() - yes calls the new
PartyConnection.confirmDeviceTakeover() (re-sends "join" with
force:true), no backs out (navigate home for video, leaveMusicParty
for music).
- Deliberately does NOT change the home page's live party list itself
(still just "Rejoindre" either way) - LobbyPartyDto is one snapshot
shared by the whole "lobby" room with no per-viewer membership info,
and giving it that is a real protocol change on its own. For video
the conflict check above already is the meaningful moment (you can't
be looking at the home page from inside your own party on the same
tab - the player is a full-screen takeover), so this doesn't cost
the feature much: Rejoindre reaches the check immediately, same
device reconnects silently, a different device asks first. See
docs/architecture.md for the fuller reasoning.
- server/test/socketGateway.test.ts, server/test/partyService.test.ts:
new tests for the conflict/takeover/reconnect paths and join()'s
idempotency; the earlier stale-connection crash regression test
updated to reach the same shared-slot shape through a real device
takeover instead of a raw PartyService.join() call (which no longer
reproduces it now that join() is idempotent).
Verified: full typecheck+test+build on both projects (39/39 server
tests passing); confirmed the "leave"/"disconnect" guard is load-
bearing by temporarily reverting it and watching the new takeover test
fail, then restoring it; deviceId.ts's localStorage persistence
checked against the real compiled bundle in a real browser (survives
a reload, stable across calls).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
parent
c14d0bc08b
commit
daab755ce9
11 changed files with 396 additions and 36 deletions
11
README.md
11
README.md
|
|
@ -20,7 +20,10 @@ including why this replaced an earlier Emby-plugin attempt (still in
|
|||
is running, it's already on the list, updating in real time.
|
||||
3. Pick a movie/episode tile and hit "Lancer une watch party" (or drill into
|
||||
a Series' seasons/episodes first) — you become the host — or hit
|
||||
"Rejoindre" on any party already listed on the home page.
|
||||
"Rejoindre" on any party already listed on the home page. Already
|
||||
connected to that same party from another device? You're asked to
|
||||
confirm before it gets disconnected — reconnecting from the same
|
||||
device (a reload, a second tab) never asks.
|
||||
4. The host's play/pause/seek drives everyone else's player automatically;
|
||||
starting from an episode queues the rest of the series so the host can
|
||||
just hit "Épisode suivant" instead of restarting a party each time. If
|
||||
|
|
@ -199,7 +202,11 @@ Then put it behind your existing reverse proxy like everything else
|
|||
client-side) — see docs/architecture.md
|
||||
- [x] Live "watch parties en cours" list (host, elapsed time, member count),
|
||||
live host handoff, no join codes
|
||||
- [x] Presence overlay (avatars) + "request pause" in the player
|
||||
- [x] Presence overlay (avatars) + a request toolbar (pause/resume) in the
|
||||
player — see docs/architecture.md
|
||||
- [x] Device detection: reconnecting from the same device is silent, a
|
||||
genuinely different device asks to confirm before disconnecting the
|
||||
other one — see docs/architecture.md
|
||||
- [x] Server-side logging of the party lifecycle (create/join/leave/destroy,
|
||||
host handoff, play/pause, pause requests) — see docs/architecture.md
|
||||
- [x] Music: dedicated player, Artist/Album/Track browsing, participative
|
||||
|
|
|
|||
|
|
@ -7,6 +7,7 @@
|
|||
//
|
||||
// No join codes: joining happens straight from the home page's live list
|
||||
// (connectLobby below) by party id - see docs/architecture.md.
|
||||
import { getDeviceId } from '../deviceId';
|
||||
import type { ItemChangedMsg, LobbyPartyDto, MusicPublicState, MusicTrackInput, PartyPublicState, PauseRequestedMsg, ResumeRequestedMsg, SocketMember } from '../types';
|
||||
|
||||
async function apiFetch<T>(path: string, options: RequestInit = {}): Promise<T> {
|
||||
|
|
@ -78,6 +79,12 @@ export interface PartyConnectionCallbacks {
|
|||
onPauseRequested?: (msg: PauseRequestedMsg) => void;
|
||||
onResumeRequested?: (msg: ResumeRequestedMsg) => void;
|
||||
onQueueUpdated?: (state: MusicPublicState) => void;
|
||||
/** This account is already actively connected to this party from a
|
||||
* DIFFERENT device (see client/src/deviceId.ts) - the socket is still
|
||||
* connected but inert (not joined to the party) until the caller either
|
||||
* calls confirmDeviceTakeover() or gives up (e.g. navigates away). See
|
||||
* SocketGateway's "join" handler. */
|
||||
onDeviceConflict?: () => void;
|
||||
onError?: (message: string) => void;
|
||||
onClose?: () => void;
|
||||
}
|
||||
|
|
@ -97,6 +104,10 @@ export interface PartyConnection {
|
|||
voteSkip(): void;
|
||||
/** Music only: no host check server-side, unlike sendPlaystate above. */
|
||||
setMusicPaused(isPaused: boolean): void;
|
||||
/** Re-sends "join" with force:true after the user confirms they want to
|
||||
* disconnect their other device and take over here - call this from
|
||||
* onDeviceConflict's handler once the user says yes. */
|
||||
confirmDeviceTakeover(): void;
|
||||
leave(): void;
|
||||
}
|
||||
|
||||
|
|
@ -107,14 +118,18 @@ export interface PartyConnection {
|
|||
* alive via closures, never gets released.
|
||||
*/
|
||||
export function connectParty(partyId: string, callbacks: PartyConnectionCallbacks = {}): PartyConnection {
|
||||
const { onMembers, onPlaystate, onItemChanged, onHostChanged, onPauseRequested, onResumeRequested, onQueueUpdated, onError, onClose } = callbacks;
|
||||
const { onMembers, onPlaystate, onItemChanged, onHostChanged, onPauseRequested, onResumeRequested, onQueueUpdated, onDeviceConflict, onError, onClose } = callbacks;
|
||||
const socket = io();
|
||||
const deviceId = getDeviceId();
|
||||
|
||||
// Reconnects (Socket.IO retries automatically) re-fire "connect", so this
|
||||
// also covers rejoining after a transient network drop, not just the
|
||||
// first connection.
|
||||
// first connection. force is deliberately NOT set here even on a
|
||||
// reconnect - a drop-and-retry re-sends the same deviceId as before, so
|
||||
// it never conflicts with itself; force only ever comes from
|
||||
// confirmDeviceTakeover() below, after a real "deviceConflict".
|
||||
socket.on('connect', () => {
|
||||
socket.emit('join', { partyId });
|
||||
socket.emit('join', { partyId, deviceId });
|
||||
});
|
||||
|
||||
if (onMembers) socket.on('members', (m) => onMembers(m as SocketMember[]));
|
||||
|
|
@ -124,6 +139,7 @@ export function connectParty(partyId: string, callbacks: PartyConnectionCallback
|
|||
if (onPauseRequested) socket.on('pauseRequested', (m) => onPauseRequested(m as PauseRequestedMsg));
|
||||
if (onResumeRequested) socket.on('resumeRequested', (m) => onResumeRequested(m as ResumeRequestedMsg));
|
||||
if (onQueueUpdated) socket.on('queueUpdated', (m) => onQueueUpdated(m as MusicPublicState)); // music parties only - see musicBar/MusicBar.ts
|
||||
if (onDeviceConflict) socket.on('deviceConflict', () => onDeviceConflict());
|
||||
if (onError) socket.on('errorMessage', (m) => onError((m as { message: string }).message));
|
||||
if (onClose) socket.on('disconnect', () => onClose());
|
||||
|
||||
|
|
@ -146,6 +162,9 @@ export function connectParty(partyId: string, callbacks: PartyConnectionCallback
|
|||
setMusicPaused(isPaused) {
|
||||
socket.emit('musicPlaystate', { isPaused });
|
||||
},
|
||||
confirmDeviceTakeover() {
|
||||
socket.emit('join', { partyId, deviceId, force: true });
|
||||
},
|
||||
leave() {
|
||||
socket.emit('leave');
|
||||
socket.disconnect();
|
||||
|
|
|
|||
40
client/src/deviceId.ts
Normal file
40
client/src/deviceId.ts
Normal file
|
|
@ -0,0 +1,40 @@
|
|||
'use strict';
|
||||
|
||||
/**
|
||||
* A persistent id for THIS browser/device, used only to tell "same account,
|
||||
* same device reconnecting" apart from "same account, a genuinely different
|
||||
* device" - see SocketGateway's "join" handler (server/src/realtime/
|
||||
* SocketGateway.ts) and docs/architecture.md. Not an identity/auth
|
||||
* mechanism - the session cookie already handles who the user is; this is
|
||||
* purely "which of that user's devices is this".
|
||||
*
|
||||
* Persisted in localStorage (not sessionStorage) so it survives a reload
|
||||
* and stays stable across tabs of the same browser - which is exactly what
|
||||
* lets two tabs of the same browser count as "the same device" (no
|
||||
* takeover prompt), while a different browser or a different computer gets
|
||||
* a fresh id (a real conflict).
|
||||
*/
|
||||
const KEY = 'watchparty_device_id';
|
||||
|
||||
let cached: string | null = null;
|
||||
|
||||
export function getDeviceId(): string {
|
||||
if (cached) return cached;
|
||||
try {
|
||||
let id = localStorage.getItem(KEY);
|
||||
if (!id) {
|
||||
id = crypto.randomUUID();
|
||||
localStorage.setItem(KEY, id);
|
||||
}
|
||||
cached = id;
|
||||
return id;
|
||||
} catch {
|
||||
// Private browsing / storage disabled: fall back to a per-page-load id.
|
||||
// Every socket "join" from this tab still sends the same value (kept in
|
||||
// `cached`), so it behaves fine as long as the page stays open - it
|
||||
// just won't survive a reload, which only means a reload always looks
|
||||
// like "a different device" to the server's conflict check above.
|
||||
cached = crypto.randomUUID();
|
||||
return cached;
|
||||
}
|
||||
}
|
||||
|
|
@ -122,6 +122,16 @@ export function joinMusicParty(partyId: string, party: PartyPublicState): void {
|
|||
conn = connectParty(partyId, {
|
||||
onMembers: renderMembers,
|
||||
onQueueUpdated: applyMusicState,
|
||||
onDeviceConflict: () => {
|
||||
// Same take-over prompt as views/player/Player.ts - see its own
|
||||
// comment. Declining here means leaving the bar entirely (there's no
|
||||
// "page" to just navigate away from, unlike the video player).
|
||||
const takeOver = confirm(
|
||||
"Tu es déjà connecté à cette party depuis un autre appareil.\n\nContinuer ici va déconnecter l'autre appareil."
|
||||
);
|
||||
if (takeOver) conn?.confirmDeviceTakeover();
|
||||
else leaveMusicParty();
|
||||
},
|
||||
onError: (message) => alert(message),
|
||||
onClose: () => {}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -456,6 +456,18 @@ function renderVideoPlayer(root: HTMLElement, partyId: string, party: PartyPubli
|
|||
onHostChanged: () => becomeHost(true),
|
||||
onPauseRequested,
|
||||
onResumeRequested,
|
||||
onDeviceConflict: () => {
|
||||
// This socket is still connected but inert (see SocketGateway's
|
||||
// "join" handler / api/party.ts) - a confirm() is fine here even
|
||||
// though it blocks: this only ever fires right after opening the
|
||||
// party, before any real playback state has built up locally to
|
||||
// interrupt.
|
||||
const takeOver = confirm(
|
||||
"Tu es déjà connecté à cette party depuis un autre appareil.\n\nContinuer ici va déconnecter l'autre appareil."
|
||||
);
|
||||
if (takeOver) conn?.confirmDeviceTakeover();
|
||||
else navigate('/home');
|
||||
},
|
||||
onError: (message) => alert(message),
|
||||
onClose: () => {}
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1534,22 +1534,124 @@ before the video player ever renders; a party's host still sees the
|
|||
settings gear open/close correctly inside that same run (see the `.hide`
|
||||
fix above).
|
||||
|
||||
## Device detection: telling a second tab apart from a second device
|
||||
|
||||
Requested directly, alongside the crash below (same underlying gap - see
|
||||
its own comment on the connection between the two): "same account, same
|
||||
device already in the party" should never offer "Rejoindre" friction at
|
||||
all, and "same account, a genuinely *different* device" should ask before
|
||||
disconnecting it, not silently steal the slot. `party.members` is
|
||||
`Map<userId, PartyMember>` - one entry per Emby account, no concept of
|
||||
*which physical device* holds it, so before this there was no way to even
|
||||
ask the question "is this the same device or not."
|
||||
|
||||
- **Identity**: `client/src/deviceId.ts` - a UUID (`crypto.randomUUID()`)
|
||||
generated once and kept in `localStorage`, not `sessionStorage`. That
|
||||
choice is what makes "device" mean "browser," not "tab": two tabs of the
|
||||
same browser share one `localStorage`, so they share one deviceId and
|
||||
never conflict with each other, while a different browser or a
|
||||
different computer always gets a fresh one - a real conflict. A
|
||||
save/read failure (private browsing, storage disabled) falls back to a
|
||||
per-page-load id, kept only in memory - correct behavior degrades to
|
||||
"every reload looks like a new device," not a crash.
|
||||
- **Protocol**: `"join"` now carries `{partyId, deviceId, force?}` instead
|
||||
of just `{partyId}` (`ClientToServerEvents`/`api/party.ts`), and
|
||||
`PartyMember` gained `deviceId: string | null` (null until a real socket
|
||||
"join" ever succeeds - a member the REST join/create routes just
|
||||
created has no live connection yet, so nothing to compare against). The
|
||||
`"join"` handler in `SocketGateway` now reasons about three cases, not
|
||||
one:
|
||||
- No existing connection, or the same `deviceId` reconnecting (a reload,
|
||||
a second tab, a dropped connection retrying) → attach silently, and if
|
||||
an old connection for that same device is still technically open,
|
||||
boot it first - it's the same physical device asking again, nothing to
|
||||
confirm.
|
||||
- A *different* `deviceId`, `force` not set → emit `"deviceConflict"` to
|
||||
the *new* socket only (never broadcast) and stop there. The socket
|
||||
stays connected but inert - not joined to the party room, the member's
|
||||
`connection` untouched - until the client either re-sends `"join"`
|
||||
with `force:true` or gives up.
|
||||
- A different `deviceId` with `force:true` (the client re-sending after
|
||||
the user confirms) → boot the old connection
|
||||
(`errorMessage` + `disconnect(true)`) and take the slot, exactly like
|
||||
the reconnect case above.
|
||||
- **Client UX**: the answer to "what should happen" was asked directly -
|
||||
a plain `confirm()` in both `Player.ts` and `MusicBar.ts`'s
|
||||
`onDeviceConflict` callback ("...continuer ici va déconnecter l'autre
|
||||
appareil"), calling the new `PartyConnection.confirmDeviceTakeover()`
|
||||
(re-emits `"join"` with `force:true`) on yes, or backing out (`navigate
|
||||
('/home')` for video, `leaveMusicParty()` for music - the bar has no
|
||||
page to just walk away from) on no. Kept as a native blocking dialog
|
||||
deliberately: this only ever fires once, right after opening a party,
|
||||
before there's any local playback state a modal could awkwardly
|
||||
interrupt.
|
||||
- **A second, independent bug closed by the same pass**: `PartyService
|
||||
.join()` used to unconditionally build a fresh member on *every* call,
|
||||
even for a userId already present - so the REST "Rejoindre" route (which
|
||||
calls this every time regardless of whether the caller is already a
|
||||
member) silently reset `connection`/`isPaused`/`deviceId` on a re-click,
|
||||
a page reload, or a retried request. `join()` is now idempotent: an
|
||||
existing member is returned as-is. This isn't just tidiness - it's the
|
||||
other half of the crash below's root cause (two connections silently
|
||||
sharing one map slot), fixed independently of the guard that stops the
|
||||
crash itself.
|
||||
- **Generalizing the crash's own fix**: `"leave"`/`"disconnect"` used to
|
||||
act on `party` (the connection's own closure variable) alone. With a
|
||||
device takeover now a real possibility, the *superseded* connection's
|
||||
own eventual `"disconnect"` (Socket.IO fires this automatically right
|
||||
after the server calls `.disconnect(true)` on it) would otherwise still
|
||||
be read as a real leave - ending the *new* connection's membership,
|
||||
wrong host handoff, a party destroyed under a still-present member. Both
|
||||
handlers now also check `party.members.get(userId)?.connection ===
|
||||
socket` before acting - only the connection actually holding the slot
|
||||
can end it.
|
||||
- **What this deliberately does *not* do**: change what the home page's
|
||||
live party list shows *before* you click "Rejoindre" - it still can't
|
||||
tell "you're already in this one, same device" from "you're already in
|
||||
this one, a different device" from "you're not in it at all," because
|
||||
`LobbyPartyDto`'s one shared snapshot (`SocketGateway.lobbySnapshot`,
|
||||
broadcast identically to the whole `"lobby"` room) carries no per-viewer
|
||||
membership at all - giving it that would mean computing and sending a
|
||||
personalized snapshot per subscriber instead of one shared broadcast, a
|
||||
real protocol change of its own, not asked for here. In practice this
|
||||
doesn't cost the feature much: for **video**, the only way to already be
|
||||
"in" a party while looking at the home page at all is a second
|
||||
tab/device (the player is a full-screen takeover on the one you're
|
||||
using - see "Music bar polish" above for why music's simpler, same-tab
|
||||
case could just be hidden client-side and video's can't), so the
|
||||
conflict check above already *is* the meaningful moment - clicking
|
||||
"Rejoindre" reaches it immediately, same-device reconnects silently,
|
||||
a different device asks first. The outcome asked for (no friction for
|
||||
your own device, a confirmation before disconnecting another) is there;
|
||||
it just resolves one click later than a home-page button label would,
|
||||
rather than needing a second, heavier protocol change to preempt it.
|
||||
|
||||
Verified with new `SocketGateway`/`PartyService` tests exercising the
|
||||
real conflict/takeover/reconnect paths (a different device gets
|
||||
`deviceConflict` and the existing connection is left untouched; `force:
|
||||
true` boots the old connection and the new one takes the slot, and its
|
||||
superseded predecessor's own later `"disconnect"` no longer destroys the
|
||||
party; the same `deviceId` reconnecting never conflicts with itself), plus
|
||||
a dedicated `PartyService.join()` idempotency test, and confirmed
|
||||
`deviceId.ts`'s `localStorage` persistence survives a real page reload
|
||||
against the compiled bundle in a real browser.
|
||||
|
||||
## A real server crash: one userId, two connections, one map slot
|
||||
|
||||
Reported directly from the running container's own crash output, not
|
||||
found by testing: `TypeError: Cannot read properties of undefined
|
||||
(reading 'isPaused')` inside `SocketGateway`'s `"playstate"` handler,
|
||||
which took the entire Node process down - every active party, not just
|
||||
one connection. `party.members` is a `Map<userId, PartyMember>` - there's
|
||||
no per-device identity at all yet (the same gap behind the "same account,
|
||||
different device" feature asked for around the same time - see the
|
||||
Home/party-list section above for the simpler, already-handled half of
|
||||
that: your own listen party no longer offers "Rejoindre" for *this* tab,
|
||||
but nothing yet distinguishes a genuinely different device sharing your
|
||||
account). Two connections for the same userId - two tabs, a second
|
||||
device, a `POST /api/parties/:id/join` retried - end up sharing that one
|
||||
map slot: `PartyService.join`/`.create` both just `members.set(userId,
|
||||
newMember)`, unconditionally overwriting whichever member was there.
|
||||
one connection. At the time this was found, `party.members` was a
|
||||
`Map<userId, PartyMember>` with no per-device identity at all - the same
|
||||
gap the "Device detection" section right above eventually closed, once
|
||||
that was actually asked for; this section describes the state of things
|
||||
before that, and the immediate fix that had to stand on its own regardless
|
||||
of whether a fuller fix ever followed. Two connections for the same
|
||||
userId - two tabs, a second device, a `POST /api/parties/:id/join`
|
||||
retried - end up sharing that one map slot: `PartyService.join`/`.create`
|
||||
both just `members.set(userId, newMember)`, unconditionally overwriting
|
||||
whichever member was there.
|
||||
|
||||
The actual crash needed one more step, and it's a real gap in
|
||||
`PartyService.leave`, not just the sharing itself: when the connection
|
||||
|
|
@ -1594,11 +1696,12 @@ throw, by temporarily reverting just the guard and watching that same
|
|||
test correctly fail before putting it back.
|
||||
|
||||
Deliberately not "fixed" by changing how `join`/`leave` share that map
|
||||
slot in this same pass - that's the real design work the fuller device-
|
||||
detection feature above still needs (identifying a device, deciding what
|
||||
"same account, different device" should actually do instead of silently
|
||||
overwriting). This pass stops it from ever being able to crash the
|
||||
server again in the meantime, regardless of how that gets resolved.
|
||||
slot in this same pass - that real design work (identifying a device,
|
||||
deciding what "same account, different device" should actually do
|
||||
instead of silently overwriting) landed separately, right above, once
|
||||
it had actually been asked for. This pass alone was enough to stop it
|
||||
from ever being able to crash the server again in the meantime,
|
||||
regardless of how - or whether - that got resolved.
|
||||
|
||||
## Known limitations
|
||||
|
||||
|
|
|
|||
|
|
@ -17,7 +17,18 @@ import type { AppServer, LeaveResult, LobbyPartyDto, Party, SocketMemberDto } fr
|
|||
* "subscribeLobby" joins the "lobby" room
|
||||
* "parties" (server -> client) full snapshot, sent on subscribe
|
||||
* and again whenever any party changes
|
||||
* "join" {partyId} joins that party's own room
|
||||
* "join" {partyId, deviceId, force?} joins that party's own room -
|
||||
* deviceId identifies which physical
|
||||
* device/browser this is (see
|
||||
* client/src/deviceId.ts); force:true
|
||||
* is a confirmed takeover, re-sent
|
||||
* after "deviceConflict" below
|
||||
* "deviceConflict" (server -> client, to the joining socket only)
|
||||
* this account is already connected
|
||||
* from a *different* deviceId - the
|
||||
* socket stays connected but inert
|
||||
* until the client re-sends "join"
|
||||
* with force:true or gives up
|
||||
* "members" (server -> client) that party's member list
|
||||
*
|
||||
* -- mediaType: 'video' parties --
|
||||
|
|
@ -73,7 +84,7 @@ class SocketGatewayClass {
|
|||
socket.emit('parties', this.lobbySnapshot());
|
||||
}));
|
||||
|
||||
socket.on('join', this.safe((msg: { partyId: string }) => {
|
||||
socket.on('join', this.safe((msg: { partyId: string; deviceId: string; force?: boolean }) => {
|
||||
const p = PartyService.getById(msg?.partyId);
|
||||
const member = p?.members.get(userId);
|
||||
if (!p || !member) {
|
||||
|
|
@ -81,8 +92,32 @@ class SocketGatewayClass {
|
|||
socket.disconnect(true);
|
||||
return;
|
||||
}
|
||||
const deviceId = msg?.deviceId || null;
|
||||
// Same account, a DIFFERENT device already actively connected as
|
||||
// this member - ask the new socket to confirm a takeover instead of
|
||||
// silently booting the other device. member.deviceId is null until
|
||||
// the first real socket "join" ever succeeds (a member just created
|
||||
// by the REST join/create routes has no live connection yet), so
|
||||
// there's nothing to conflict with the very first time.
|
||||
if (member.connection && member.deviceId && deviceId && member.deviceId !== deviceId && !msg?.force) {
|
||||
socket.emit('deviceConflict', { partyId: p.id });
|
||||
// This socket stays connected but inert: not joined to the party
|
||||
// room, member.connection untouched - until the client re-sends
|
||||
// "join" with force:true or gives up.
|
||||
return;
|
||||
}
|
||||
// Either a confirmed takeover (force:true) or the same device
|
||||
// reconnecting (reload, dropped connection) - boot whatever
|
||||
// connection currently holds the slot before taking its place, so
|
||||
// ITS eventual "disconnect" can't be mistaken for a real leave (see
|
||||
// the connection===socket guard on "leave"/"disconnect" below).
|
||||
if (member.connection && member.connection !== socket) {
|
||||
member.connection.emit('errorMessage', { message: 'Tu as rejoint depuis un autre appareil.' });
|
||||
member.connection.disconnect(true);
|
||||
}
|
||||
party = p;
|
||||
member.connection = socket;
|
||||
member.deviceId = deviceId;
|
||||
socket.join(party.id);
|
||||
this.broadcastMembers(io, party);
|
||||
}));
|
||||
|
|
@ -198,17 +233,29 @@ class SocketGatewayClass {
|
|||
}));
|
||||
|
||||
socket.on('leave', this.safe(() => {
|
||||
if (!party) return;
|
||||
// Only actually leave if THIS socket is still the connection the
|
||||
// member map has on file. A device takeover (see "join" above)
|
||||
// reassigns member.connection to the new socket and force-
|
||||
// disconnects the old one; without this guard the old socket's own
|
||||
// "leave"/"disconnect" would incorrectly end the NEW connection's
|
||||
// membership (wrong host handoff, party destroyed under a still-
|
||||
// present member, etc).
|
||||
if (party && party.members.get(userId)?.connection === socket) {
|
||||
this.applyLeave(io, party, userId);
|
||||
}
|
||||
party = null;
|
||||
}));
|
||||
|
||||
// Fires on a clean "leave", a dead-connection timeout (Socket.IO's
|
||||
// own heartbeat - see docs/architecture.md), or any other way the
|
||||
// own heartbeat - see docs/architecture.md), a device takeover's
|
||||
// forced disconnect of the OLD connection, or any other way the
|
||||
// socket goes away - the one cleanup path that always runs, unlike
|
||||
// relying on the client to always get a chance to emit "leave" first.
|
||||
// Same stale-connection guard as "leave" above.
|
||||
socket.on('disconnect', this.safe(() => {
|
||||
if (party) this.applyLeave(io, party, userId);
|
||||
if (party && party.members.get(userId)?.connection === socket) {
|
||||
this.applyLeave(io, party, userId);
|
||||
}
|
||||
}));
|
||||
});
|
||||
}
|
||||
|
|
|
|||
|
|
@ -71,7 +71,23 @@ class PartyServiceClass {
|
|||
return this.partiesById.get(partyId) || null;
|
||||
}
|
||||
|
||||
/**
|
||||
* Idempotent for a userId already in `party`: returns the *existing*
|
||||
* member as-is rather than resetting it. This matters because the REST
|
||||
* "Rejoindre" flow calls this every time regardless of whether the
|
||||
* caller is already a member - re-clicking it, a page reload, a second
|
||||
* device/tab, a stale retry. Blindly overwriting used to reset
|
||||
* connection/isPaused/deviceId on every one of those, which is exactly
|
||||
* what let two connections for one userId silently share (and lose
|
||||
* track of) one map slot - see docs/architecture.md's server-crash
|
||||
* section. A genuinely new member still goes through here fresh; only
|
||||
* the "already present" case is now a no-op. Device-aware conflict
|
||||
* handling for an *already-connected* member lives in SocketGateway's
|
||||
* "join" handler instead - this method has no socket to reason about.
|
||||
*/
|
||||
join(party: Party, userId: string, userName: string): PartyMember {
|
||||
const existing = party.members.get(userId);
|
||||
if (existing) return existing;
|
||||
const member = this.newMember(userId, userName);
|
||||
party.members.set(userId, member);
|
||||
party.lastActivityUtc = Date.now();
|
||||
|
|
@ -201,6 +217,7 @@ class PartyServiceClass {
|
|||
userId,
|
||||
userName,
|
||||
connection: null,
|
||||
deviceId: null,
|
||||
isPaused: true,
|
||||
positionTicks: 0,
|
||||
lastUpdatedUtc: Date.now()
|
||||
|
|
|
|||
|
|
@ -21,7 +21,12 @@ import type { Server as IoServer, Socket as IoSocket } from 'socket.io';
|
|||
|
||||
export interface ClientToServerEvents {
|
||||
subscribeLobby: () => void;
|
||||
join: (msg: { partyId: string }) => void;
|
||||
/** deviceId: persisted client-side (see client/src/deviceId.ts) - lets
|
||||
* the "join" handler tell "this same account, same device reconnecting"
|
||||
* apart from "this same account, a genuinely different device" (see
|
||||
* SocketGateway's own comment). force: true re-sends this after the
|
||||
* user confirmed a "deviceConflict" takeover. */
|
||||
join: (msg: { partyId: string; deviceId: string; force?: boolean }) => void;
|
||||
playstate: (msg: { isPaused: boolean; positionTicks: number }) => void;
|
||||
next: () => void;
|
||||
voteSkip: () => void;
|
||||
|
|
@ -58,6 +63,12 @@ export interface ServerToClientEvents {
|
|||
hostChanged: () => void;
|
||||
pauseRequested: (msg: { userId: string; userName: string }) => void;
|
||||
resumeRequested: (msg: { userId: string; userName: string }) => void;
|
||||
/** Sent only to the joining socket itself (never broadcast) when this
|
||||
* account is already actively connected from a different deviceId -
|
||||
* see SocketGateway's "join" handler. The socket stays connected but
|
||||
* inert (not attached to the party) until the client either re-sends
|
||||
* "join" with force:true or gives up. */
|
||||
deviceConflict: (msg: { partyId: string }) => void;
|
||||
errorMessage: (msg: { message: string }) => void;
|
||||
}
|
||||
|
||||
|
|
@ -170,6 +181,11 @@ export interface PartyMember {
|
|||
userId: string;
|
||||
userName: string;
|
||||
connection: AppSocket | null;
|
||||
/** Whichever deviceId last successfully attached `connection` - null
|
||||
* until the first real socket "join" (a member freshly created by the
|
||||
* REST join/create routes has no live connection yet, so nothing to
|
||||
* compare a new one against). See SocketGateway's "join" handler. */
|
||||
deviceId: string | null;
|
||||
isPaused: boolean;
|
||||
positionTicks: number;
|
||||
lastUpdatedUtc: number;
|
||||
|
|
|
|||
|
|
@ -52,6 +52,25 @@ describe('PartyService', () => {
|
|||
assert.equal(PartyService.getById(party.id), null);
|
||||
});
|
||||
|
||||
it('join() is idempotent: re-joining an existing member returns it as-is instead of resetting it', () => {
|
||||
const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
|
||||
const bob = PartyService.join(party, 'u-bob', 'Bob');
|
||||
bob.isPaused = false;
|
||||
bob.positionTicks = 12345;
|
||||
bob.deviceId = 'device-1';
|
||||
|
||||
// The REST "Rejoindre" route (and a second device/tab's socket "join")
|
||||
// call this every time regardless of whether the caller is already a
|
||||
// member - this must not reset any of the live state just set above
|
||||
// (see this method's own comment for the crash that used to cause).
|
||||
const bobAgain = PartyService.join(party, 'u-bob', 'Bob');
|
||||
assert.equal(bobAgain, bob, 'must be the exact same member object, not a fresh one');
|
||||
assert.equal(bobAgain.isPaused, false, 'a re-join must not reset live state');
|
||||
assert.equal(bobAgain.positionTicks, 12345);
|
||||
assert.equal(bobAgain.deviceId, 'device-1');
|
||||
assert.equal(party.members.size, 2, 'still exactly one entry for bob, not a duplicate');
|
||||
});
|
||||
|
||||
it('removeIdleParties sweeps a party whose lastActivityUtc is past the cutoff', () => {
|
||||
const party = PartyService.create('u-dave', 'Dave', 'm1', 'Un Film', '/img/m1', null, 'video');
|
||||
party.lastActivityUtc = Date.now() - 5 * 60 * 60 * 1000;
|
||||
|
|
|
|||
|
|
@ -259,7 +259,7 @@ describe('SocketGateway', () => {
|
|||
assert.equal(lobbyRefreshCount(), 2, 'advancing the queue resets position/pause for the new item - the home page needs that immediately, not just whenever some other event next refreshes it');
|
||||
});
|
||||
|
||||
it('a stale connection\'s next heartbeat after the map slot it shared gets deleted does not crash the process (regression - see docs/architecture.md)', (t) => {
|
||||
it('a stale connection\'s next heartbeat after a device takeover destroys the party does not crash the process (regression - see docs/architecture.md)', (t) => {
|
||||
const crashed: unknown[] = [];
|
||||
t.mock.method(console, 'error', (..._args: unknown[]) => { crashed.push(_args); });
|
||||
|
||||
|
|
@ -269,17 +269,18 @@ describe('SocketGateway', () => {
|
|||
|
||||
const socketA = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketA as unknown as AppSocket);
|
||||
socketA.trigger('join', { partyId: party.id });
|
||||
socketA.trigger('join', { partyId: party.id, deviceId: 'device-A' });
|
||||
|
||||
// A second join for the same userId (a second device/tab, before any
|
||||
// real per-device identity - see docs/architecture.md) re-runs
|
||||
// PartyService.join, which overwrites the one map slot this userId
|
||||
// occupies - orphaning socketA's own member record from the map even
|
||||
// though socketA itself is still very much "connected".
|
||||
PartyService.join(party, 'u-alice', 'Alice');
|
||||
// A confirmed device takeover (see the "join" handler below) hands
|
||||
// socketA's map slot to socketB - the exact "two connections for the
|
||||
// same userId share one slot" shape the original crash needed, now
|
||||
// reached through the real takeover flow instead of a raw
|
||||
// PartyService.join() call (that method is idempotent now - see its
|
||||
// own test in partyService.test.ts - so a second call to it would no
|
||||
// longer reproduce this).
|
||||
const socketB = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketB as unknown as AppSocket);
|
||||
socketB.trigger('join', { partyId: party.id });
|
||||
socketB.trigger('join', { partyId: party.id, deviceId: 'device-B', force: true });
|
||||
|
||||
// socketB (currently occupying the slot) leaves - the only member, so
|
||||
// the party is destroyed; hostUserId is never reassigned since
|
||||
|
|
@ -295,4 +296,73 @@ describe('SocketGateway', () => {
|
|||
assert.doesNotThrow(() => socketA.trigger('playstate', { isPaused: true, positionTicks: 0 }));
|
||||
assert.equal(crashed.length, 0, 'should be a clean early return (the specific guard), not the generic safe() net silently swallowing an actual throw');
|
||||
});
|
||||
|
||||
it('a second device for the same account gets a deviceConflict instead of silently taking over', () => {
|
||||
const io = fakeIo();
|
||||
attach(io);
|
||||
const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
|
||||
|
||||
const socketA = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketA as unknown as AppSocket);
|
||||
socketA.trigger('join', { partyId: party.id, deviceId: 'device-A' });
|
||||
|
||||
const socketB = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketB as unknown as AppSocket);
|
||||
const roomEmitsBefore = io.roomEmits.length;
|
||||
socketB.trigger('join', { partyId: party.id, deviceId: 'device-B' });
|
||||
|
||||
assert.ok(socketB.emitted.some(([event]) => event === 'deviceConflict'), 'the new device should be told about the conflict');
|
||||
assert.equal(party.members.get('u-alice')?.connection, socketA, 'the existing connection must stay in place until the conflict is confirmed');
|
||||
assert.equal(io.roomEmits.length, roomEmitsBefore, 'a conflicting join must not broadcast "members" - the socket never actually joined');
|
||||
assert.ok(!socketA.emitted.some(([event]) => event === 'errorMessage'), 'the existing device must not be kicked yet');
|
||||
});
|
||||
|
||||
it('re-joining with force:true confirms the takeover: kicks the old device, attaches the new one', () => {
|
||||
const io = fakeIo();
|
||||
attach(io);
|
||||
const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
|
||||
|
||||
const socketA = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketA as unknown as AppSocket);
|
||||
socketA.trigger('join', { partyId: party.id, deviceId: 'device-A' });
|
||||
|
||||
const socketB = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketB as unknown as AppSocket);
|
||||
socketB.trigger('join', { partyId: party.id, deviceId: 'device-B' }); // conflict - the client would show a confirm() here
|
||||
socketB.trigger('join', { partyId: party.id, deviceId: 'device-B', force: true }); // user confirmed
|
||||
|
||||
assert.ok(socketA.emitted.some(([event]) => event === 'errorMessage'), 'the old device should be told it got disconnected');
|
||||
assert.equal(party.members.get('u-alice')?.connection, socketB, 'the new device now holds the slot');
|
||||
assert.equal(party.members.get('u-alice')?.deviceId, 'device-B');
|
||||
|
||||
// The old connection's own eventual "disconnect" - Socket.IO fires this
|
||||
// automatically after a server-side .disconnect(true), simulated here
|
||||
// manually like this file's other tests - must NOT be mistaken for a
|
||||
// real leave: it no longer holds the member's connection slot (see the
|
||||
// "join" handler's own comment on why, and the guard on "leave"/
|
||||
// "disconnect" below).
|
||||
socketA.trigger('disconnect');
|
||||
assert.equal(PartyService.getById(party.id), party, 'the party must survive the superseded connection\'s disconnect');
|
||||
assert.ok(party.members.has('u-alice'), 'the member must still be present, still attached to socketB');
|
||||
});
|
||||
|
||||
it('the same device reconnecting (matching deviceId) takes over silently - no deviceConflict', () => {
|
||||
const io = fakeIo();
|
||||
attach(io);
|
||||
const party = PartyService.create('u-alice', 'Alice', 'm1', 'Un Film', '/img/m1', null, 'video');
|
||||
|
||||
const socketA = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketA as unknown as AppSocket);
|
||||
socketA.trigger('join', { partyId: party.id, deviceId: 'device-A' });
|
||||
|
||||
// e.g. a page reload on the same browser: a fresh socket, but the same
|
||||
// deviceId persisted in localStorage (see client/src/deviceId.ts).
|
||||
const socketA2 = fakeSocket('u-alice');
|
||||
io.trigger('connection', socketA2 as unknown as AppSocket);
|
||||
socketA2.trigger('join', { partyId: party.id, deviceId: 'device-A' });
|
||||
|
||||
assert.ok(!socketA2.emitted.some(([event]) => event === 'deviceConflict'), 'the same device should never conflict with itself');
|
||||
assert.equal(party.members.get('u-alice')?.connection, socketA2);
|
||||
assert.equal(party.members.get('u-alice')?.deviceId, 'device-A');
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Reference in a new issue