pro/im_webapp
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(web): code review round 2 -- critical session/connection races

Browse Source 
- RequireAuth: a page reload lost the session even with a valid token
  sitting in sessionStorage -- myUserId is never persisted (by design)
  but nothing ever re-derived it from the reusable login token. Added
  connection.ts's restoreSession(), which also fixes the race the first
  attempt at this ran into live: React 19 StrictMode double-invokes the
  restore effect in dev, and Tinode.loginToken() isn't idempotent (second
  call on an already-authenticated connection gets 409'd), so both the
  connect() and the login itself needed module-level in-flight dedup,
  not just a per-effect cancelled flag
- topics.ts: same dedup pattern for openConversation/closeConversation --
  StrictMode's mount->cleanup->remount always overlapped an open with
  itself, and a leave() landing before its own subscribe() resolved was
  silently no-op'ing instead of detaching
- LOGGING_ENABLED was unconditionally on, including prod builds --
  Tinode's verbose logging prints the raw login secret and full message
  content to the console
- imPost/imPatch/imPut/imDelete could silently drop the Idempotency-Key
  header for a caller that passes its own headers (object-spread order
  bug, dormant today since no call site does this yet)
- DetailColumn's getFriends()/getBlocks() only ever read page 1 despite
  fetchAllPages existing specifically to prevent this bug class
- availableActions() now defers to the server's message_policy.
  supported_actions when present instead of only a client-side time
  window, and exposes recallRemainingSeconds for the context menu's
  countdown label

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
Your Name
2026-09-08 04:48:28 +08:00
co-authored by Claude Sonnet 5
parent 1bd8d7f804
commit d3a0eb0dbc
12 changed files with 181 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
web/src/lib/tinode/connection.ts
+43 -5
View File
@@ -9,7 +9,7 @@
// useSyncExternalStore-based hooks actually subscribe to.
import { Tinode, type LoginCtrl } from 'tinode-sdk';
import { API_KEY, APP_NAME, LOGGING_ENABLED } from '@/lib/config';
import { API_KEY, APP_NAME, DEFAULT_HOST, LOGGING_ENABLED } from '@/lib/config';
import { useSessionStore } from '@/stores/session';
export type ConnectionStatus = 'disconnected' | 'connecting' | 'connected' | 'authenticated';
@@ -138,7 +138,7 @@ function installTenantHiPatch(t: Tinode) {
function createTinode(): Tinode {
const tenant = useSessionStore.getState().tenant;
const host = tenant?.host || 'web.118im.com';
const host = tenant?.host || DEFAULT_HOST;
const secure = tenant?.secure ?? true;
const t = new Tinode({ appName: APP_NAME, host, apiKey: API_KEY, transport: 'ws', secure, persist: false });
t.setHumanLanguage('zh');
@@ -198,11 +198,49 @@ export function resetTinode() {
setStatus('disconnected');
}
export async function connect(): Promise<void> {
/**
* Tracked so two overlapping callers (e.g. React 19 StrictMode running an effect twice in dev, or
* RequireAuth's session restore and a LoginPage submit racing) share one in-flight connect() instead
* of each calling `t.connect()` -- the SDK doesn't dedupe this itself, and a second `t.connect()`
* while the first is still mid-handshake tears down and replaces that first WebSocket outright
* (confirmed live: "WebSocket is closed before the connection is established"), which can abort a
* `loginToken()` that was riding on the first connection.
*/
let connectPromise: Promise<void> | null = null;
export function connect(): Promise<void> {
const t = getTinode();
if (t.isConnected()) return;
if (t.isConnected()) return Promise.resolve();
if (connectPromise) return connectPromise;
setStatus('connecting');
await t.connect();
connectPromise = t.connect().finally(() => {
connectPromise = null;
});
return connectPromise;
}
/**
* Restores a session from a token written to sessionStorage by a previous login (RequireAuth.tsx,
* on mount, when `myUserId` hasn't survived a same-tab reload). Deduped the same way connect() is
* above and for the same reason: `Tinode.loginToken()` isn't idempotent (a second login on an
* already-authenticated connection gets rejected with `409 already authenticated`), so React 19
* StrictMode running RequireAuth's restore effect twice in dev would otherwise send the SAME token
* twice and treat the resulting 409 as "this token is bad" -- discarding a token that had, a moment
* earlier on the very same connection, just been accepted.
*/
let restorePromise: Promise<string | null> | null = null;
export function restoreSession(token: string): Promise<string | null> {
if (restorePromise) return restorePromise;
restorePromise = (async () => {
await connect();
const ctrl = await getTinode().loginToken(token);
if (ctrl.code >= 300) throw new Error('session token rejected');
return getTinode().getCurrentUserID() || null;
})().finally(() => {
restorePromise = null;
});
return restorePromise;
}
export function getConnectionStatus(): ConnectionStatus {
web/src/lib/tinode/topics.ts
+36 -12
View File
@@ -98,6 +98,17 @@ export function prefetchPreview(topicName: string): void {
});
}
/**
* The SDK has no per-topic in-flight tracking of its own: `Topic.subscribe()` only short-circuits
* once `_attached` is already true, so two overlapping calls (React 19 StrictMode's mount->cleanup->
* remount runs every effect twice in dev, so openConversation() always overlaps with itself once per
* open; ChatListColumn's prefetchPreview and this both racing for the same just-deep-linked topic is
* the same shape in prod) would otherwise send two `{sub}` packets, and a `leave()` that lands before
* the first `subscribe()` resolves just rejects ("Cannot leave inactive topic") and no-ops instead of
* detaching. Tracked here so every caller for the same topic name shares one in-flight promise.
*/
const inFlightSubscribe = new Map<string, Promise<Topic>>();
/**
* Opens a topic for full conversation view: fetches a page of history and wires live updates.
* Uses withData(undefined, undefined, limit), NOT withLaterData(limit) -- if prefetchPreview already
@@ -108,20 +119,33 @@ export function prefetchPreview(topicName: string): void {
* at all until the user's first send (avoiding server-side subscription state for chats opened but
* never used) -- not yet replicated here since the new-conversation picker flow doesn't exist yet.
*/
export async function openConversation(topicName: string): Promise<Topic> {
const topic = getTinode().getTopic(topicName);
wireTopicCallbacks(topic);
if (topic.isSubscribed()) {
await topic.leave(false);
}
const query = topic.startMetaQuery().withLaterDesc().withLaterSub().withAux().withData(undefined, undefined, MESSAGES_PAGE);
await topic.subscribe(query.build());
return topic;
export function openConversation(topicName: string): Promise<Topic> {
const existing = inFlightSubscribe.get(topicName);
if (existing) return existing;
const promise = (async () => {
const topic = getTinode().getTopic(topicName);
wireTopicCallbacks(topic);
if (topic.isSubscribed()) {
await topic.leave(false);
}
const query = topic.startMetaQuery().withLaterDesc().withLaterSub().withAux().withData(undefined, undefined, MESSAGES_PAGE);
await topic.subscribe(query.build());
return topic;
})();
inFlightSubscribe.set(topicName, promise);
promise.finally(() => {
if (inFlightSubscribe.get(topicName) === promise) inFlightSubscribe.delete(topicName);
});
return promise;
}
/** Soft-leave (detach, not unsubscribe) -- cached messages stay in the SDK's local buffer. */
export function closeConversation(topicName: string): void {
getTinode().getTopic(topicName).leave(false).catch(() => {});
/** Soft-leave (detach, not unsubscribe) -- cached messages stay in the SDK's local buffer. Waits out
* any in-flight openConversation() for this same topic first, so a cleanup that fires before the
* subscribe it's undoing has even resolved doesn't just silently no-op (see inFlightSubscribe above). */
export async function closeConversation(topicName: string): Promise<void> {
const pending = inFlightSubscribe.get(topicName);
if (pending) await pending.catch(() => {});
await getTinode().getTopic(topicName).leave(false).catch(() => {});
}
let chatListSnapshot: Contact[] | null = null;