Improve chat bootstrap, performance#3168
Conversation
📝 WalkthroughWalkthroughThis PR adds a 20-second timeout to the chat API client, implements bootstrap caching with TTL and concurrent request deduplication for Mattermost sessions, and ensures the bootstrap cache is cleared during logout and refresh flows across application and settings containers. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client/UI
participant Provider as Mattermost Provider
participant Cache as Bootstrap Cache
participant API as Mattermost API
rect rgba(100, 150, 200, 0.5)
Note over Client,API: First Request — Cache Miss
Client->>Provider: bootstrapMattermostSession(account, pinCode)
Provider->>Cache: Check cache for username
Cache-->>Provider: No entry found
Provider->>Provider: Check inFlightBootstrap[username]
Provider-->>Provider: No in-flight request
Provider->>API: GET /api/mattermost/bootstrap
API-->>Provider: Bootstrap data + token
Provider->>Cache: Store result with TTL
Provider->>Provider: Set chatApiToken
Provider-->>Client: Return bootstrap data
end
rect rgba(150, 200, 100, 0.5)
Note over Client,API: Concurrent Request — Cache Hit & Deduplication
Client->>Provider: bootstrapMattermostSession(account, pinCode)
Client->>Provider: bootstrapMattermostSession(account, pinCode)
Provider->>Cache: Check cache for username
Cache-->>Provider: Cached entry exists & not expired
Provider->>Provider: Reapply setChatApiToken
Provider-->>Client: Return cached data (both requests)
end
rect rgba(200, 150, 100, 0.5)
Note over Client,API: Logout — Cache Cleared
Client->>Provider: clearMattermostBootstrapCache()
Provider->>Cache: Clear all cached entries
Provider->>Provider: Clear inFlightBootstrap
Provider->>Provider: Increment bootstrapGeneration
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/providers/chat/mattermost.ts (1)
89-92: Minor: Duplicate username validation.This check duplicates the validation in
resolveMattermostTokens(lines 18-20). While harmless as defensive programming, consider consolidating to avoid the duplicate error path.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/providers/chat/mattermost.ts` around lines 89 - 92, The duplicate username validation in src/providers/chat/mattermost.ts should be removed to avoid duplicated error paths: since resolveMattermostTokens already validates and throws when no user is present, eliminate the redundant block that reads const username = currentAccount?.name; if (!username) { throw new Error('No authenticated user found'); } and instead rely on the username/token returned by resolveMattermostTokens (or move the single validation into a shared helper); update any local uses to consume the resolved username/token from resolveMattermostTokens (e.g., ensure functions like resolveMattermostTokens are called before using username).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/providers/chat/mattermost.ts`:
- Around line 89-92: The duplicate username validation in
src/providers/chat/mattermost.ts should be removed to avoid duplicated error
paths: since resolveMattermostTokens already validates and throws when no user
is present, eliminate the redundant block that reads const username =
currentAccount?.name; if (!username) { throw new Error('No authenticated user
found'); } and instead rely on the username/token returned by
resolveMattermostTokens (or move the single validation into a shared helper);
update any local uses to consume the resolved username/token from
resolveMattermostTokens (e.g., ensure functions like resolveMattermostTokens are
called before using username).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 4c650112-b99e-4066-89af-484c4c4888c3
📒 Files selected for processing (4)
src/config/chatApi.tssrc/providers/chat/mattermost.tssrc/screens/application/container/applicationContainer.tsxsrc/screens/settings/container/settingsContainer.tsx
Summary by CodeRabbit
Bug Fixes
Performance
Reliability