diff --git a/packages/calling/src/CallingClient/registration/register.test.ts b/packages/calling/src/CallingClient/registration/register.test.ts index 097cf2fb78f..e1a1f11c1e9 100644 --- a/packages/calling/src/CallingClient/registration/register.test.ts +++ b/packages/calling/src/CallingClient/registration/register.test.ts @@ -727,6 +727,110 @@ describe('Registration Tests', () => { }); }); + describe('restorePreviousRegistration 429 handling tests', () => { + beforeEach(() => { + jest.useFakeTimers(); + }); + + afterEach(() => { + jest.clearAllTimers(); + jest.clearAllMocks(); + jest.useRealTimers(); + }); + + it('should schedule retry when 429 with retry-after < 60 seconds during reconnect', async () => { + restartSpy = jest.spyOn(reg, 'restartRegistration'); + reg.setActiveMobiusUrl(mobiusUris.primary[0]); + const failurePayload429Small = ({ + statusCode: 429, + body: 'SOMETHING RANDOM', + headers: { + 'retry-after': 30, + }, + }); + webex.request + .mockRejectedValueOnce(failurePayload429Small) + .mockResolvedValueOnce(successPayload) + .mockRejectedValueOnce(failurePayload429Small) + .mockResolvedValueOnce(successPayload); + + await reg.reconnectOnFailure('SOMETHING RANDOM'); // This call is being used to set the retry-after value + + await reg.reconnectOnFailure('SOMETHING RANDOM'); // This call is being used to trigger the retry + jest.advanceTimersByTime(40 * SEC_TO_MSEC_MFACTOR); + await flushPromises(); + + expect(restartSpy).toHaveBeenCalledTimes(1); + expect(restartSpy).toHaveBeenCalledWith('SOMETHING RANDOM'); + }); + + it('should try backup servers when 429 with retry-after >= 60 seconds on primary during reconnect', async () => { + // Setup: Register successfully with primary first + const attemptRegistrationWithServersSpy = jest.spyOn(reg, 'attemptRegistrationWithServers'); + reg.setActiveMobiusUrl(mobiusUris.primary[0]); + const failurePayload429Small = ({ + statusCode: 429, + body: 'SOMETHING RANDOM', + headers: { + 'retry-after': 100, + }, + }); + webex.request + .mockRejectedValueOnce(failurePayload429Small) + .mockResolvedValueOnce(successPayload) + .mockRejectedValueOnce(failurePayload429Small) + .mockResolvedValueOnce(successPayload); + + await reg.reconnectOnFailure('SOMETHING RANDOM'); // This call is being used to trigger the retry + jest.advanceTimersByTime(40 * SEC_TO_MSEC_MFACTOR); + await flushPromises(); + + expect(attemptRegistrationWithServersSpy).toHaveBeenCalledTimes(2); + expect(attemptRegistrationWithServersSpy).toHaveBeenNthCalledWith(1, 'SOMETHING RANDOM', [ + mobiusUris.primary[0], + ]); + // Immediately try backup servers when retry-after >= 60 seconds on primary + expect(attemptRegistrationWithServersSpy).toHaveBeenNthCalledWith( + 2, + 'SOMETHING RANDOM', + mobiusUris.backup + ); + }); + it('should restart registration with primary if we get 429 while on backup', async () => { + // Setup: Register successfully with primary first + restartSpy = jest.spyOn(reg, 'restartRegistration'); + const attemptRegistrationWithServersSpy = jest.spyOn(reg, 'attemptRegistrationWithServers'); + reg.setActiveMobiusUrl(mobiusUris.backup[0]); + const failurePayload429Small = ({ + statusCode: 429, + body: 'SOMETHING RANDOM', + headers: { + 'retry-after': 100, + }, + }); + webex.request + .mockRejectedValueOnce(failurePayload429Small) + .mockResolvedValueOnce(successPayload) + .mockRejectedValueOnce(failurePayload429Small) + .mockResolvedValueOnce(successPayload); + + await reg.reconnectOnFailure('SOMETHING RANDOM'); // This call is being used to trigger the retry + jest.advanceTimersByTime(40 * SEC_TO_MSEC_MFACTOR); + await flushPromises(); + + expect(attemptRegistrationWithServersSpy).toHaveBeenCalledTimes(2); + expect(attemptRegistrationWithServersSpy).toHaveBeenNthCalledWith(1, 'SOMETHING RANDOM', [ + mobiusUris.backup[0], + ]); + // Immediately try primary servers when retry-after >= 60 seconds on backup + expect(restartSpy).toHaveBeenCalledTimes(1); + expect(restartSpy).toHaveBeenCalledWith('SOMETHING RANDOM'); + expect(attemptRegistrationWithServersSpy).toHaveBeenNthCalledWith(2, 'SOMETHING RANDOM', [ + mobiusUris.primary[0], + ]); + }); + }); + describe('Registration failover tests', () => { it('verify unreachable primary with reachable backup servers', async () => { jest.useFakeTimers(); @@ -872,7 +976,10 @@ describe('Registration Tests', () => { // delete should be successful global.fetch = jest.fn(() => Promise.resolve({json: () => mockDeleteResponse})) as jest.Mock; - postRegistrationSpy.mockRejectedValue(failurePayload429Two); + // Mock to fail twice with 429 (once for executeFailback, once for restorePreviousRegistration) + postRegistrationSpy + .mockRejectedValueOnce(failurePayload429Two) + .mockRejectedValueOnce(failurePayload429Two); /* Wait for failback to be triggered. */ jest.advanceTimersByTime( @@ -892,11 +999,14 @@ describe('Registration Tests', () => { failurePayload429Two.headers['retry-after'], 'executeFailback' ); - expect(reg.failback429RetryAttempts).toBe(0); + // After handling 429 during failback, the counter is incremented to 1 + expect(reg.failback429RetryAttempts).toBe(1); expect(reg.getStatus()).toBe(RegistrationStatus.INACTIVE); expect(restoreSpy).toBeCalledOnceWith(REG_429_RETRY_UTIL); - expect(restartSpy).toBeCalledOnceWith(REG_429_RETRY_UTIL); - expect(reg.failbackTimer).toBe(undefined); + // restartRegistration is not called immediately because 429 with retry-after < 60 + // schedules a delayed retry instead + expect(restartSpy).not.toBeCalled(); + expect(reg.failbackTimer).not.toBe(undefined); // Timer is set in handle429Retry expect(reg.rehomingIntervalMin).toBe(DEFAULT_REHOMING_INTERVAL_MIN); expect(reg.rehomingIntervalMax).toBe(DEFAULT_REHOMING_INTERVAL_MAX); }); diff --git a/packages/calling/src/CallingClient/registration/register.ts b/packages/calling/src/CallingClient/registration/register.ts index 885cad3dfc1..8de7eec83bb 100644 --- a/packages/calling/src/CallingClient/registration/register.ts +++ b/packages/calling/src/CallingClient/registration/register.ts @@ -207,9 +207,32 @@ export class Registration implements IRegistration { */ private async restorePreviousRegistration(caller: string): Promise { let abort = false; - + // Testing: + // 4XX + // 429 big n small value + // 404 + // 500 if (this.activeMobiusUrl) { abort = await this.attemptRegistrationWithServers(caller, [this.activeMobiusUrl]); + if (this.retryAfter) { + if (this.retryAfter < RETRY_TIMER_UPPER_LIMIT) { + // If retry-after is less than threshold, honor it and schedule retry + setTimeout(async () => { + await this.restartRegistration(caller); + }, this.retryAfter * 1000); + } else if ( + this.primaryMobiusUris.includes(this.activeMobiusUrl) && + this.backupMobiusUris.length > 0 + ) { + // If we are using primary and got 429, switch to backup + abort = await this.attemptRegistrationWithServers(caller, this.backupMobiusUris); + } else { + // If we are using backup and got 429, restart registration + this.restartRegistration(caller); + } + + return true; + } } return abort;