fix(federation): DM first message not working #37048
Conversation
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughA 200 ms asynchronous delay was introduced before invoking startJoiningRoom in ee/packages/federation-matrix/src/api/_matrix/invite.ts, replacing the previous immediate call. No exported or public API signatures were changed. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant IH as InviteHandler
participant T as Timer (200ms)
participant JR as startJoiningRoom
C->>IH: Receive invite event
note right of IH: Previous flow: IH -> JR immediately
IH->>T: setTimeout(200ms, callback)
T-->>IH: timeout fires (200ms later)
IH->>JR: startJoiningRoom({inviteEvent, user, room, state})
JR-->>IH: Join process continues
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## chore/federation-backup #37048 +/- ##
===========================================================
+ Coverage 69.75% 69.77% +0.02%
===========================================================
Files 3035 3035
Lines 103431 103431
Branches 18383 18385 +2
===========================================================
+ Hits 72146 72171 +25
+ Misses 29405 29380 -25
Partials 1880 1880
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
rodrigok
force-pushed
the
fix/federation-dm-first-message
branch
from
e2e8a0c to
81c1c24
Compare
rodrigok
force-pushed
the
fix/federation-dm-first-message
branch
from
81c1c24 to
a7b858f
Compare
rodrigok
changed the base branch from
use-published-package
to
chore/federation-backup
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ee/packages/federation-matrix/src/api/_matrix/invite.ts (1)
134-144: Fix the backoff timing (ms vs sec) — current logic retries almost immediately, then explodes to multi‑day delaysThe units are mixed up. You log ms but calculate seconds, call setTimeout with seconds (treated as ms), and pass ms back as seconds for the next iteration. This causes a 25 ms retry instead of 25 s, then jumps to days on subsequent attempts. Patch below keeps seconds for the state, ms for timers, and caps at 625 s as per the comment.
async function runWithBackoff(fn: () => Promise<void>, delaySec = 5) { try { await fn(); } catch (e) { - const delay = delaySec === 625 ? 625 : delaySec ** 2; - console.log(`error occurred, retrying in ${delay}ms`, e); - setTimeout(() => { - runWithBackoff(fn, delay * 1000); - }, delay); + const nextDelaySec = delaySec >= 625 ? 625 : delaySec ** 2; + const delayMs = delaySec * 1000; + console.log(`error occurred, retrying in ${delayMs}ms`, e); + setTimeout(() => { + void runWithBackoff(fn, nextDelaySec); + }, delayMs); } }
🧹 Nitpick comments (2)
ee/packages/federation-matrix/src/api/_matrix/invite.ts (2)
325-333: Avoid hardcoded 200 ms “magic sleep”; make it configurable (or justify necessity)If the intent is to decouple from the request or wait for persistence, a fixed 200 ms is brittle. Either rely on runWithBackoff (no delay) or make the initial delay a named, configurable constant.
- setTimeout(() => { + setTimeout(() => { void startJoiningRoom({ inviteEvent, user: ourUser, room, state, }); - }, 200); + }, INVITE_INITIAL_JOIN_DELAY_MS);Add near the top of the file (or a config module):
// Initial defer before attempting the first join; allows eventual consistency after invite processing. const INVITE_INITIAL_JOIN_DELAY_MS = Number(process.env.FEDERATION_INVITE_INITIAL_JOIN_DELAY_MS ?? 200);Please confirm the root cause that necessitated the delay; if it’s eventual consistency, consider replacing this with a readiness check and relying solely on the (fixed) backoff.
285-287: Remove unnecessaryasyncfromstartJoiningRoomThe function doesn’t
awaitanything and always returns immediately; droppingasyncavoids returning an unused Promise.-async function startJoiningRoom(...opts: Parameters<typeof joinRoom>) { +function startJoiningRoom(...opts: Parameters<typeof joinRoom>) { void runWithBackoff(() => joinRoom(...opts)); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
ee/packages/federation-matrix/src/api/_matrix/invite.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: 🔨 Test Unit / Unit Tests
- GitHub Check: 🔎 Code Check / TypeScript
- GitHub Check: 🔎 Code Check / Code Lint
- GitHub Check: 📦 Meteor Build - coverage
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
2 participants
Related to RocketChat/homeserver#212
Proposed changes (including videos or screenshots)
Issue(s)
Steps to test or reproduce
Further comments
Summary by CodeRabbit