Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: prevent auto-dropping already accepted or rejected calls #1619

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
+40 −30
Open

fix: prevent auto-dropping already accepted or rejected calls #1619

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c5fb1b9
fix: prevent auto-dropping already accepted or rejected calls
myandrienko Dec 11, 2024
ff04987
additional guard against dropping not ringing call
myandrienko Dec 11, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
70 changes: 40 additions & 30 deletions packages/client/src/Call.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,16 +342,18 @@ export class Call {
);

this.leaveCallHooks.add(
// watch for auto drop cancellation
createSubscription(this.state.callingState$, (callingState) => {
// cancel auto-drop when call is
createSubscription(this.state.session$, (session) => {
if (!this.ringing) return;
if (
callingState === CallingState.JOINED ||
callingState === CallingState.JOINING ||
callingState === CallingState.LEFT
) {
Comment on lines -348 to -352
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cancelling auto-drop on calling state change is no longer required, since it's covered by two other checks:

  1. Auto-drop is cancelled if the call is either accepted or rejected by the current user.
  2. Auto-drop is a no-op if calling state changed to anything other than RINGING.

clearTimeout(this.dropTimeout);
this.dropTimeout = undefined;

const receiverId = this.clientStore.connectedUser?.id;
if (!receiverId) return;

const isAcceptedByMe = Boolean(session?.accepted_by[receiverId]);
const isRejectedByMe = Boolean(session?.rejected_by[receiverId]);

if (isAcceptedByMe || isRejectedByMe) {
this.cancelAutoDrop();
}
}),
);
Expand Down Expand Up @@ -2000,28 +2002,36 @@ export class Call {
* Applicable only for ringing calls.
*/
private scheduleAutoDrop = () => {
clearTimeout(this.dropTimeout);
this.leaveCallHooks.add(
createSubscription(this.state.settings$, (settings) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Previously, a subscription on call settings was set up when scheduling auto-drop. I don't see a reason to set up the subscription here.

Call settings should be up-to-date at this point; and in case call settings were somehow update mid-ringing, this subscription didn't take updated timeouts into account correctly anyway.

So, to simplify things, we are just setting up a timeout using current call settings.

if (!settings) return;
// ignore if the call is not ringing
if (this.state.callingState !== CallingState.RINGING) return;

const timeoutInMs = this.isCreatedByMe
? settings.ring.auto_cancel_timeout_ms
: settings.ring.incoming_call_timeout_ms;

// 0 means no auto-drop
if (timeoutInMs <= 0) return;
this.cancelAutoDrop();

const settings = this.state.settings;
if (!settings) return;
// ignore if the call is not ringing
if (this.state.callingState !== CallingState.RINGING) return;

const timeoutInMs = this.isCreatedByMe
? settings.ring.auto_cancel_timeout_ms
: settings.ring.incoming_call_timeout_ms;

// 0 means no auto-drop
if (timeoutInMs <= 0) return;

this.dropTimeout = setTimeout(() => {
// the call might have stopped ringing by this point,
// e.g. it was already accepted and joined
if (this.state.callingState !== CallingState.RINGING) return;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An additional guard to prevent dropping a call that is no longer RINGING (e.g. was accepted or rejected on the current device/tab).

this.leave({ reject: true, reason: 'timeout' }).catch((err) => {
this.logger('error', 'Failed to drop call', err);
});
}, timeoutInMs);
};

clearTimeout(this.dropTimeout);
this.dropTimeout = setTimeout(() => {
this.leave({ reject: true, reason: 'timeout' }).catch((err) => {
this.logger('error', 'Failed to drop call', err);
});
}, timeoutInMs);
}),
);
/**
* Cancels a scheduled auto-drop timeout.
*/
private cancelAutoDrop = () => {
clearTimeout(this.dropTimeout);
this.dropTimeout = undefined;
};

/**
Expand Down
Loading