diff --git a/src/utils.js b/src/utils.js index b0727dc..f339973 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1373,107 +1373,119 @@ export const reqUserStateChange = async (newState) => { } }; -// add userId to meta counter +// ────────────────────────────────────────────────────────────────────────── +// Lobby matching — why this is structured as a single transaction +// ────────────────────────────────────────────────────────────────────────── +// A previous implementation split lobby matching into two transactions: one +// to append the user to meta.counter, and a second to check if the counter +// was full and reset it. Between those two transactions, additional users +// could join the counter, so counter.length could exceed groupSize. The +// reset check used strict equality (counter.length === groupSize), so any +// overflow state would fall through to a log-and-do-nothing branch and the +// lobby would permanently deadlock (e.g. 3+ users joining near-simultaneously +// would never get paired). +// +// This version collapses add + capacity check + reset into ONE transaction, +// and gates the add on capacity. That maintains the invariant: +// +// after every committed write, counter.length < groupSize +// +// Because the invariant holds, overflow is structurally impossible, and the +// `=== groupSize` check on the closing user is safe. When a user's add +// brings the counter to exactly groupSize, that same transaction captures +// the dyad and resets the counter to [] atomically — no other client can +// ever observe the "full" state, so there's no window for a second client +// to also try to initialize the group. +// +// Firestore's optimistic concurrency guarantees transaction retries on +// conflict, so concurrent joiners are serialized: each retry reads the +// committed post-state of the previous commit. Each attempt therefore does +// exactly one of: +// - "duplicate": user already present in counter → no-op +// - "full": counter already at capacity (invariant violation — this +// should never fire in normal operation; it's an assertion +// that surfaces external writes or bugs loudly rather than +// silently deadlocking) +// - "waiting": user appended, counter still below capacity +// - "closed": user appended, counter hit capacity, reset + dyad captured +// +// Only the "closed" caller calls initGroup. Non-closing users learn about +// their new group via the onSnapshot listener on their participant doc +// (see App.svelte:188), which fires when initGroup writes groupId onto their +// user record and then transitively subscribes them to the group doc. +// +// Note: `dyad` is returned from the transaction callback rather than being +// captured in a closure. Firestore may retry the callback on conflict, and +// a closure variable set on attempt #1 could leak stale state into the +// post-transaction code if attempt #2 takes a different branch. Returning +// the result from runTransaction avoids that class of bug entirely — each +// retry produces its own fresh result object. +// ────────────────────────────────────────────────────────────────────────── + +// add userId to meta counter; if this add closes the lobby, initialize the group export const reqMetaDocChange = async (userId) => { const metaDocRef = doc(db, metaCollectionName, 'data'); - // update meta doc + let result; try { - await runTransaction(db, async (transaction) => { - - // Get the latest data, rather than relying on the store + result = await runTransaction(db, async (transaction) => { const document = await transaction.get(metaDocRef); if (!document.exists()) { - throw "Document does not exist!"; + throw new Error("Meta doc does not exist"); } - // Freshest data const { counter } = document.data(); - const data = { counter: [...counter, userId] }; - // Add the user to the counter if they're not already in it - if (!counter.includes(userId)) { - await transaction.update(metaDocRef, data); - } else { - console.log("Ignoring duplicate request"); + // Dedupe: user already in the lobby (e.g. double-click, reconnect). + if (counter.includes(userId)) { + console.log(`User ${userId} already in lobby counter; ignoring duplicate request`); + return { status: "duplicate" }; } - }); - } catch (error) { - console.error(`Error updating meta doc for userId: ${userId}`, error); - } - // Use helper function to run a second transaction that checks the counter length and - // actually performs the state change if appropriate - await checkIfResetCounter(userId); -}; -// helper funtion to run check for meta doc change -// every time user joins, check if counter is full -const checkIfResetCounter = async () => { - // Get latest counter - let counter; - - const metaDocRef = doc(db, metaCollectionName, 'data'); - try { - await runTransaction(db, async (transaction) => { - // Get the latest data, rather than relying on the store - const document = await transaction.get(metaDocRef); - if (!document.exists()) { - throw "Document does not exist!"; + // Invariant check: counter should never be at or above groupSize after + // any committed write. If it is, something external has touched it. + // Refuse to add this user and surface loudly rather than silently + // deadlocking or joining a phantom group. + if (counter.length >= globalVars.groupSize) { + console.warn( + `LOBBY_FULL invariant violation: counter.length=${counter.length} >= ` + + `groupSize=${globalVars.groupSize}. Not adding user ${userId}.` + ); + return { status: "full" }; } - // Get latest counter - counter = document.data().counter; - // group size met - if (counter.length === globalVars.groupSize) { - // if you're the last user to join the group, - // you initialize the group doc - console.log('Resetting counter...'); - // reset counter - const obj = {}; - obj["counter"] = []; - await transaction.update(metaDocRef, obj); - } else { - console.log("No need to reset. Still waiting for users to join..."); + const next = [...counter, userId]; + + if (next.length === globalVars.groupSize) { + // This user's add closes the lobby. Atomically reset the counter + // and capture the dyad for initGroup below. + transaction.update(metaDocRef, { counter: [] }); + console.log(`Lobby closed by ${userId}; dyad=[${next}]`); + return { status: "closed", dyad: next }; } + + // Otherwise just append and wait for more joiners. + transaction.update(metaDocRef, { counter: next }); + console.log( + `User ${userId} added to lobby; still waiting for ` + + `${globalVars.groupSize - next.length} more` + ); + return { status: "waiting" }; }); } catch (error) { - console.error(`Error verifying meta doc change`, error); + console.error(`Error updating meta doc for userId: ${userId}`, error); + return; } - if (counter.length === globalVars.groupSize) { - console.log('Initializing group...'); - // initialize group - await initGroup(counter); - } else if (counter.length > globalVars.groupSize) { - console.log(`Counter length ${counter.length} exceeds group size`); - console.log("counter", counter) - - // // Loop over the array two items at a time - // for (let i = 0; i < counter.length; i += 2) { - // // Check if there is only one item left - // if (counter.length === 1) { - // console.log("1 person left:", counter[0]); - - // // reset counter - // // const obj = {}; - // // obj["counter"] = counter; - // // await transaction.update(metaDocRef, obj); - // } else { - // // Get two items from the array - // let userA = counter[0]; - // let userB = counter[1]; - // console.log("2 people moving:", userA, userB) - - // // Call initGroup with two items - // await initGroup([userA, userB]); - // // Remove the processed items from the array - // counter.splice(0, 2); - // console.log("updated coutner", counter); - // } - // } - } else { - console.log(`Still waiting for ${globalVars.groupSize - counter.length} requests...`); + if (result && result.status === "closed") { + // Only the closing user runs initGroup. Other users in the dyad are + // notified via their participant-doc onSnapshot listener (App.svelte) + // once initGroup writes their groupId. + await initGroup(result.dyad); + } else if (result && result.status === "full") { + // Optional: surface to caller so the UI can route to a failure/wait + // screen. For now we just return; callers can inspect logs. + return; } - }; // Save vignette information (no chat participants only)