Skip to content

Fix presence auto re-enter error message missing clientId (RTP17e) - #2314

Merged
ttypic merged 2 commits into
ably:mainfrom
cpruijsen:fix/issue-2209
Oct 20, 2026
Merged

ttypic merged 2 commits into
ably:mainfrom
cpruijsen:fix/issue-2209

Conversation

@cpruijsen

@cpruijsen cpruijsen commented Sep 28, 2026 •

Copy link
Copy Markdown
Contributor

Summary

RealtimePresence._ensureMyMembersPresent() interpolates entry.clientId into the 91004 ErrorInfo on the channel update that auto re-enter already emits when the ENTER was NACKed. RTP17e requires that message to name the member that failed to re-enter.

The UTS test that asserts reason.message includes my-client now runs without RUN_DEVIATIONS, and the matching deviations.md entry is removed.

Decision: put entry.clientId into the current 'Presence auto re-enter failed' string, quoted like the MICRO log in the same function. Alternative: copy Dart (Automatic re-entry failed for clientId "...") or .NET (Cannot automatically re-enter {clientId} on channel {channelName}). The spec and the UTS test only require the clientId to appear, so keeping the existing phrase is the smallest change. Can switch to a sibling SDK's string.

Independent of #2301.

Fixes #2209

Test plan

  • npx mocha --no-config --exit --require tsx/cjs --timeout 30000 --grep "RTP17e - failed re-entry emits UPDATE with error" test/uts/realtime/unit/presence/realtime_presence_reentry.test.ts passes without RUN_DEVIATIONS (Node 20)
  • Same grep fails if the source string is reverted to the fixed message (expected 'Presence auto re-enter failed' to include 'my-client')
  • The other five tests in realtime_presence_reentry.test.ts still pass

Summary by CodeRabbit

  • Bug Fixes
    • Error messages for failed automatic presence re-entry now include the affected member’s client ID, providing clearer diagnostic context for the impacted connection.
    • Added coverage to verify this behavior without requiring a deviation-based test skip.

@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 76ff4bfc-1947-44c4-8981-3075ccdc076d
📥 Commits

Reviewing files that changed from the base of the PR and between d6d9041 and 4946088.

📒 Files selected for processing: 1
  • test/uts/deviations.md
💤 Files with no reviewable changes: 1
  • test/uts/deviations.md

Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.

Walkthrough

The automatic presence re-entry failure message now includes the affected clientId. The RTP17e test no longer has a RUN_DEVIATIONS-based skip, and the related deviation entry was removed.

Changes

Presence Re-entry Error Message

Layer / File(s) Summary
Update and validate re-entry error message
src/common/lib/client/realtimepresence.ts, test/uts/realtime/unit/presence/realtime_presence_reentry.test.ts, test/uts/deviations.md
The failure message now includes the affected clientId. The RTP17e test runs without the conditional skip, and its deviation entry was removed.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~5 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: ttypic

Merge Risk: ⚪ Minimal · up to 49460

Failed automatic presence re-entry now identifies the affected client, and the targeted test covers the required UPDATE behavior. No merge-blocking issue was found.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: adding the missing clientId to the presence auto re-entry error message. It also identifies the related RTP17e requirement.
Linked Issues check Passed Issue #2209 requires RTP17e failed automatic re-entry to emit error 91004 with a message that includes the failed member clientId. The whole-PR diff updates `RealtimePresence._ensureMyMembersPresent()…
Out of Scope Changes check Passed The whole-PR diff contains one production message change and two matching UTS maintenance changes. Removing the RTP17e deviation documentation and test skip directly enables issue #2209 compliance. No…
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1…
Full details: Linked Issues check

Explanation

Issue #2209 requires RTP17e failed automatic re-entry to emit error 91004 with a message that includes the failed member clientId. The whole-PR diff updates RealtimePresence._ensureMyMembersPresent() to build Presence auto re-enter failed for clientId " + entry.clientId + ", while preserving code 91004. The diff also removes the deviation skip from the RTP17e test, so the existing clientId assertion runs normally.

Full details: Out of Scope Changes check

Explanation

The whole-PR diff contains one production message change and two matching UTS maintenance changes. Removing the RTP17e deviation documentation and test skip directly enables issue #2209 compliance. No unrelated changes appear in the diff.

Full details: Docstring Coverage

Explanation

No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 1 files.

✨ Finishing Touches: 💡 1
🛠️ Fix failing CI checks: 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

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

A rabbit saw the clientId glow,
The re-entry message tells it so.
The skipped test now runs bright,
The deviation left the site.
Hops and checks now align just right.

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

@ttypic ttypic left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM, thanks for contribution!

RTP17e requires the 91004 ErrorInfo on a failed automatic re-enter to
name the member. Interpolate entry.clientId into the existing message,
unskip the UTS assertion, and drop the resolved deviations.md entry.
@ttypic
ttypic merged commit 4c4f9de into ably:main Oct 20, 2026
7 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Presence re-entry error message does not include clientId (RTP17e)

2 participants