Skip to content

Improve chat bootstrap, performance#3168

Merged
feruzm merged 1 commit intodevelopmentfrom
boot
Apr 10, 2026
Merged

Improve chat bootstrap, performance#3168
feruzm merged 1 commit intodevelopmentfrom
boot

Conversation

@feruzm
Copy link
Copy Markdown
Member

@feruzm feruzm commented Apr 10, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Added request timeout protection to prevent indefinite API request hanging that could cause application freezes.
  • Performance

    • Introduced session caching to reduce redundant bootstrap network requests.
    • Added request deduplication to collapse concurrent initialization requests for the same user.
  • Reliability

    • Enhanced session cleanup during logout to prevent accidental reuse of stale cached sessions.

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 10, 2026

📝 Walkthrough

Walkthrough

This 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

Cohort / File(s) Summary
Chat API Configuration
src/config/chatApi.ts
Added 20,000ms timeout to Axios client instance, causing requests exceeding the limit to fail rather than hang indefinitely.
Mattermost Bootstrap Caching
src/providers/chat/mattermost.ts
Implemented per-username bootstrap result caching with short-lived TTL, concurrent request deduplication via in-flight Promise reuse, and a new exported clearMattermostBootstrapCache(username?) function. Added generation counter to prevent cache population after logout. Function now requires currentAccount.name and throws if absent.
Logout & Refresh Integration
src/screens/application/container/applicationContainer.tsx, src/screens/settings/container/settingsContainer.tsx
Updated logout and session refresh flows to explicitly clear both the shared chat API token and Mattermost bootstrap cache, preventing cached bootstrap state from restoring logged-out sessions.

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
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • Fix PAT and chat api #3160: Modifies chat API token handling (setChatApiToken/getChatApiTokenOwner) and updates Mattermost bootstrap to pass username; overlaps with this PR's token-management code paths and bootstrap flow changes.

Poem

🐰 A cache blooms with TTL so bright,
Deduplicating requests with all our might,
Timeouts tick at twenty seconds keen,
Bootstrap dreams, now lean and clean! ✨

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'Improve chat bootstrap, performance' is partially related to the changeset. It references 'chat bootstrap,' which is a core change (caching and deduplication in bootstrapMattermostSession), but omits the critical addition of request timeouts and uses a vague second term 'performance' without specifics. Consider a more specific title like 'Add timeout to chat API and cache Mattermost bootstrap' or 'Add request timeout and Mattermost bootstrap caching' to clearly convey both main improvements.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch boot

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 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

📥 Commits

Reviewing files that changed from the base of the PR and between 8fbdd01 and 413a50a.

📒 Files selected for processing (4)
  • src/config/chatApi.ts
  • src/providers/chat/mattermost.ts
  • src/screens/application/container/applicationContainer.tsx
  • src/screens/settings/container/settingsContainer.tsx

@feruzm feruzm merged commit 989e8c4 into development Apr 10, 2026
2 checks passed
@feruzm feruzm deleted the boot branch April 10, 2026 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant