Skip to content

fix: on() method now reads from correct subscription map for peerConnectionEvents#454

Merged
muke1908 merged 2 commits into
muke1908:masterfrom
yashcreator11:fix/on-method-wrong-subscription-map
May 19, 2026
Merged

fix: on() method now reads from correct subscription map for peerConnectionEvents#454
muke1908 merged 2 commits into
muke1908:masterfrom
yashcreator11:fix/on-method-wrong-subscription-map

Conversation

@yashcreator11
Copy link
Copy Markdown
Contributor

What does this PR do?

Fixes the bug reported in #453

Root Cause

The on() method in ChatE2EE was always reading from
this.subscriptions for the duplicate check, even when
the listener belonged to callSubscriptions.

Fix

Changed one line in service/src/sdk.ts:

// Before ❌
const sub = this.subscriptions.get(listener);

// After ✅
const sub = subscriptions.get(listener);

Testing

All existing 75 tests pass with no failures.

Fixes #453

…ectionEvents

- on() was always reading from this.subscriptions for duplicate check
- even when listener belonged to callSubscriptions
- this caused call event callbacks to never be deduplicated
- changed this.subscriptions.get(listener) to subscriptions.get(listener)

Fixes muke1908#453
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a bug in ChatE2EE.on() where duplicate-listener checks for peerConnectionEvents were performed against the wrong subscription map, causing call event callbacks to not be deduplicated.

Changes:

  • Update on() to read the existing subscription set from the selected subscriptions map (either subscriptions or callSubscriptions) instead of always reading from this.subscriptions.
Comments suppressed due to low confidence (1)

service/src/sdk.ts:219

  • Log message has a typo: Skpping should be Skipping (or similar) to avoid confusing/ungrep-able logs.
            if (sub.has(callback)) {
                loggerWithCount.log(`Skpping, subscription: ${listener}`);
                return;

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@sonarqubecloud
Copy link
Copy Markdown

@muke1908 muke1908 merged commit 103f9b3 into muke1908:master May 19, 2026
1 of 3 checks passed
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.

bug: on() method in ChatE2EE always deduplicates against wrong subscription map

3 participants