fix(calling): update call keepalive retry logic #4557

rarajes2 · 2025-10-31T05:14:44Z

COMPLETES #< CAI-7258 >

This pull request addresses

Adds the call keepalive error handling scenarios for 4xx and 5xx cases where for 4xx cases call cleanup happens and for 5xx keepalive retries are done based on the value received in the response header retry-after followed by fallback value if it's not present.

by making the following changes

Normal keepalive for call will be 10 minutes but if receives 5xx response then it will schedule the re-tries based on the after-retry header value and fallback to 30 seconds

Vidcast - https://app.vidcast.io/share/670a12d0-1b16-4561-8d86-f34df10e3a63

Change Type

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to change)
Documentation update
Tooling change
Internal code refactor

The following scenarios were tested

< ENUMERATE TESTS PERFORMED, WHETHER MANUAL OR AUTOMATED >

Tested the following with the burp suite (to update the response header)

For 4xx cases the call was being cleaned up
For 5xx cases with retry-after, it honors the value if provided else fallback to 30 sec retry interval
Tries max of 4 retries and if all fails it stops the retries
Failed 3 keepalives and then let 1 pass which did reset the counter and again letting the keepalive fail for 4 times stops sending further keepalive request

The GAI Coding Policy And Copyright Annotation Best Practices

I certified that

I have read and followed contributing guidelines
I discussed changes with code owners prior to submitting this pull request
I have not skipped any automated checks
All existing and new tests passed
I have updated the documentation accordingly

Make sure to have followed the contributing guidelines before submitting.

Kesari3008

Haven't looked at the test files for now, because there are comments for code changes.

packages/calling/src/CallingClient/calling/call.ts

packages/calling/src/common/Utils.ts

packages/calling/src/CallingClient/calling/call.test.ts

adhmenon

Is it possible to test this with the agent desktop?
Just want to ensure it does not break the flow.

packages/calling/src/CallingClient/calling/call.test.ts

adhmenon

Approved.
But let us test once with agent desktop just to be safe.

rarajes2 · 2025-11-13T13:58:43Z

packages/calling/src/CallingClient/calling/call.test.ts

+    expect(emitSpy).toHaveBeenCalledWith(CALL_EVENT_KEYS.DISCONNECT, call.getCorrelationId());
+  });
+
+  it('session refresh 403 ends the call without emitting error', async () => {


403 case is update here.

packages/calling/src/common/Utils.ts

packages/calling/src/CallingClient/calling/call.ts

packages/calling/src/CallingClient/calling/call.test.ts

rarajes2 · 2025-11-18T06:43:27Z

packages/calling/src/CallingClient/calling/call.ts

-            setTimeout(() => {
-              /* We first post the status and then recursively call the handler which
-               * starts the timer again
-               */
-              this.postStatus();
-              this.sendCallStateMachineEvt({type: 'E_CALL_ESTABLISHED'});
-            }, interval * 1000);


This setTimeout is not required as it was causing the this.postStatus() call to be made twice - once here and next on invoking the handleCallEstablished through state machine.

Now we are only calling handleCallEstablished through state machine immediately as the handleCallEstablished makes the keepalive call inside the setInterval which adds the required delay and keeps the counter consistent.

Kesari3008

Approving the PR but there are still significant changes needed. Please address these

Kesari3008 · 2025-11-18T11:56:59Z

packages/calling/src/CallingClient/calling/call.ts

+          this.emit(CALL_EVENT_KEYS.DISCONNECT, this.getCorrelationId());
+          this.callKeepaliveRetryCount = 0;
+
+          return;


We shouldn't skip logs uploading for final error cases, this return would cause that

Kesari3008 · 2025-11-18T12:49:31Z

packages/calling/src/CallingClient/calling/call.ts

-              this.postStatus();
-              this.sendCallStateMachineEvt({type: 'E_CALL_ESTABLISHED'});
-            }, interval * 1000);
+            this.callKeepaliveRetryCount += 1;


We didn't need to remove the setTimeout. Retry logic was implemented in a way that if it fails, we were immediately trying one more time and schedule the timeout to try it again after specific interval. Even if we wanted to avoid the postStatus being sent twice, we should just remove postStatus and keep setting the state machine to established with a interval timeout to trigger the keepalive retry

Even if we wanted to avoid the postStatus being sent twice, we should just remove postStatus and keep setting the state machine to established with a interval timeout to trigger the keepalive retry

This logic is already there. I have removed the unnecessary setTimeout and postStatus, instead I am directly invoking the logic with the state machine. Please check the new logic. If required we can discuss over a call

Kesari3008 · 2025-11-18T13:10:04Z

packages/calling/src/CallingClient/calling/call.ts

-              this.postStatus();
-              this.sendCallStateMachineEvt({type: 'E_CALL_ESTABLISHED'});
-            }, interval * 1000);
+            this.callKeepaliveRetryCount += 1;


With this logic with the first keepalive retry itself, count will start increasing

This is only being increased in case of error.

Kesari3008 · 2025-11-18T13:21:37Z

packages/calling/src/common/Utils.test.ts

+      endSpy
+    );
+
+    expect(emitted).toBe(true);


Add check for for abort being false for non-final error cases

Kesari3008 · 2025-11-18T13:22:22Z

packages/calling/src/common/Utils.test.ts

+    expect(retrySpy).not.toHaveBeenCalled();
+  });
+
+  it('503 during keepalive with retry-after does not emit and retries with interval', async () => {


it should say does not emit what

Changed to -> 503 during keepalive with retry-after does not invoke emitterCb and retries with interval

Kesari3008 · 2025-11-18T13:25:39Z

packages/calling/src/common/Utils.test.ts

+      logObj.file
+    );
+
+    expect(emitted).toBe(true);


we should be check for accurate emitter callback where we emit error event instead of plain setting it true or false. Or just pass jest.fn() and check for it's invocation

This check ensures that the passed emitterCb is being invoked. We don't need to make it complicated.

Kesari3008 · 2025-11-18T13:35:08Z

packages/calling/src/CallingClient/calling/call.test.ts

+
+    expect(clearInterval).toHaveBeenCalledTimes(1);
+    expect(funcSpy).toBeCalledTimes(1);
+    expect(emitSpy).toHaveBeenCalledWith(CALL_EVENT_KEYS.DISCONNECT, call.getCorrelationId());


we should add expect to check for end API being invoked, media connection getting closed and call getting cleared from callCollection

Wouldn't that make it an integration test? There would be a separate unit test for that already present

Kesari3008 · 2025-11-18T13:37:32Z

packages/calling/src/CallingClient/calling/call.test.ts

+      call['handleCallEstablished']({} as CallEvent);
+    }
+
+    jest.advanceTimersByTime(DEFAULT_SESSION_TIMER);


we should only advance here as per retyr after and not default session timer right

First time invocation of postStatus will only happen after the DEFAULT_SESSION_TIMER

Kesari3008 · 2025-11-18T13:39:55Z

packages/calling/src/CallingClient/calling/call.test.ts

+    jest.spyOn(call, 'postStatus').mockRejectedValue(errorPayload);
+
+    // Manually set the retry count to 3 so it becomes 4 after increment
+    call['callKeepaliveRetryCount'] = 3;


We should let the code run 4 times and count getting increased on its own. And add expect for postStatus being invoked 4 times too

rarajes2 added 6 commits October 22, 2025 11:12

fix(calling): fix the ExtendedError usage in logs and UTs

84540e7

fix(calling): improve the error logs and fix the UTs

dfd63d1

fix(calling): address review comments

c8a7837

fix(calling): warn log error messages in register file

072c1fc

fix(calling): update call keepalive retry logic

ceae37c

Merge branch 'next' into keepalive-retries-CAI-7258

e916248

rarajes2 requested a review from a team as a code owner October 31, 2025 05:14

rarajes2 marked this pull request as draft October 31, 2025 05:15

rarajes2 added 3 commits November 4, 2025 15:13

fix(calling): retry callback param fix

177ac5c

Merge branch 'next' into keepalive-retries-CAI-7258

5b99655

fix(calling): add unit tests for call keepalive error handling scenarios

d004169

rarajes2 added the validated If the pull request is validated for automation. label Nov 5, 2025

rarajes2 marked this pull request as ready for review November 5, 2025 05:36

fix(calling): add 30 sec of default timer for failing 5xx cases

69c271e

rarajes2 changed the title ~~update call keepalive retry logic~~ fix(calling): update call keepalive retry logic Nov 5, 2025

Kesari3008 requested changes Nov 6, 2025

View reviewed changes

adhmenon reviewed Nov 7, 2025

View reviewed changes

packages/calling/src/CallingClient/calling/call.test.ts Show resolved Hide resolved

fix(calling): address review comments

b87b34b

adhmenon approved these changes Nov 11, 2025

View reviewed changes

rarajes2 and others added 3 commits November 12, 2025 14:33

fix(calling): fix failing unit test

05435f0

Merge branch 'next' into keepalive-retries-CAI-7258

16df087

fix(calling): add abort to the the handleCallError

6816c90

rarajes2 commented Nov 13, 2025

View reviewed changes

Kesari3008 requested changes Nov 13, 2025

View reviewed changes

rarajes2 and others added 2 commits November 18, 2025 12:08

fix(calling): fix the keepalive retry counter logic

ebe16e4

Merge branch 'next' into keepalive-retries-CAI-7258

5fb140f

rarajes2 commented Nov 18, 2025

View reviewed changes

Kesari3008 approved these changes Nov 18, 2025

View reviewed changes

fix(calling): reset the call keepalive counter after successful response

e044e2d

rarajes2 force-pushed the keepalive-retries-CAI-7258 branch from e2e01ce to e044e2d Compare November 18, 2025 14:47

rarajes2 and others added 2 commits November 18, 2025 21:29

fix(calling): improve unit tests. submit logs for final error too

ff57334

Merge branch 'next' into keepalive-retries-CAI-7258

9280ace

rarajes2 enabled auto-merge (squash) November 18, 2025 16:00

rarajes2 merged commit d9fa606 into webex:next Nov 18, 2025
11 checks passed

fix(calling): update call keepalive retry logic #4557

fix(calling): update call keepalive retry logic #4557

Conversation

rarajes2 commented Oct 31, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

COMPLETES #< CAI-7258 >

This pull request addresses

by making the following changes

Change Type

The following scenarios were tested

The GAI Coding Policy And Copyright Annotation Best Practices

I certified that

Uh oh!

Kesari3008 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

adhmenon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

adhmenon left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kesari3008 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rarajes2 Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

rarajes2 commented Oct 31, 2025 •

edited

Loading

rarajes2 Nov 18, 2025 •

edited

Loading