diff --git a/packages/@webex/webex-core/src/lib/services-v2/services-v2.ts b/packages/@webex/webex-core/src/lib/services-v2/services-v2.ts index d6b19b5f5ab..49f7a7c0118 100644 --- a/packages/@webex/webex-core/src/lib/services-v2/services-v2.ts +++ b/packages/@webex/webex-core/src/lib/services-v2/services-v2.ts @@ -1366,23 +1366,29 @@ const Services = WebexPlugin.extend({ }, /** - * Await any in-flight credentials refresh, then flip `services.ready` so - * `webex.ready` can fire. Closes the parallel-refresh window: if a credential - * refresh is in flight when initial catalog collection settles, we must not - * signal ready until the refresh has resolved - otherwise downstream - * consumers may observe `canAuthorize`/token state that is about to change - * under them. + * Await any in-flight credentials refresh until the startup deadline, then + * flip `services.ready` so `webex.ready` can fire. * * @private + * @param {Promise} [startupDeadline] * @returns {Promise} */ - async _finalizeReady(): Promise { + async _finalizeReady(startupDeadline?: Promise): Promise { const {credentials} = this.webex; if (credentials && credentials.isRefreshing) { - await new Promise((resolve) => { - credentials.once('change:isRefreshing', resolve); + let onChangeIsRefreshing = () => {}; + const refreshSettled = new Promise((resolve) => { + onChangeIsRefreshing = resolve; + credentials.once('change:isRefreshing', onChangeIsRefreshing); + + if (!credentials.isRefreshing) { + resolve(); + } }); + + await (startupDeadline ? Promise.race([refreshSettled, startupDeadline]) : refreshSettled); + credentials.off('change:isRefreshing', onChangeIsRefreshing); } this.ready = true; @@ -1480,25 +1486,26 @@ const Services = WebexPlugin.extend({ // catalogs. We listen for 'loaded' instead of 'ready' because `services.ready` // now blocks `webex.ready` - listening to 'ready' would deadlock. this.listenToOnce(this.webex, 'loaded', async () => { + const initTimeoutMs = this.webex.config?.services?.catalogInitTimeout; + let cancelStartupDeadline = () => {}; + const startupDeadline = new Promise((resolve) => { + const timeout = setTimeout(resolve, initTimeoutMs); + + cancelStartupDeadline = () => clearTimeout(timeout); + }); + const finalizeReady = () => + this._finalizeReady(startupDeadline).finally(cancelStartupDeadline); + const warmed = await this._loadCatalogFromCache(); if (warmed) { catalog.isReady = true; - await this._finalizeReady(); + await finalizeReady(); return; } const {supertoken} = this.webex.credentials; - - // Race init against a hard timeout so a hung request never leaves - // `services.ready` false forever - that would stall `webex.ready` and - // leave consumers waiting on it indefinitely. Timeout is configurable via - // `config.services.catalogInitTimeout` (defaults to 15s in config). - const initTimeoutMs = this.webex.config?.services?.catalogInitTimeout; - const initServiceCatalogsTimeout = new Promise((_, reject) => { - setTimeout( - () => reject(new Error(`services: init timed out after ${initTimeoutMs}ms`)), - initTimeoutMs - ); + const initServiceCatalogsTimeout = startupDeadline.then(() => { + throw new Error(`services: init timed out after ${initTimeoutMs}ms`); }); // Validate if the supertoken exists. @@ -1513,7 +1520,7 @@ const Services = WebexPlugin.extend({ `services: failed to init initial services when credentials available, ${error?.message}` ); }) - .finally(() => this._finalizeReady()); + .finally(finalizeReady); } else { const {email} = this.webex.config; @@ -1527,7 +1534,7 @@ const Services = WebexPlugin.extend({ `services: failed to init initial services when no credentials available, ${error?.message}` ); }) - .finally(() => this._finalizeReady()); + .finally(finalizeReady); // Handle fresh login: 'loaded' fires before OAuth completes, so listen // for `canAuthorize` flipping true and then collect the postauth catalog. diff --git a/packages/@webex/webex-core/src/lib/services/services.js b/packages/@webex/webex-core/src/lib/services/services.js index c3a94c54fa7..3a615a3f909 100644 --- a/packages/@webex/webex-core/src/lib/services/services.js +++ b/packages/@webex/webex-core/src/lib/services/services.js @@ -1396,23 +1396,29 @@ const Services = WebexPlugin.extend({ }, /** - * Await any in-flight credentials refresh, then flip `services.ready` so - * `webex.ready` can fire. Closes the parallel-refresh window: if a credential - * refresh is in flight when initial catalog collection settles, we must not - * signal ready until the refresh has resolved - otherwise downstream - * consumers may observe `canAuthorize`/token state that is about to change - * under them. + * Await any in-flight credentials refresh until the startup deadline, then + * flip `services.ready` so `webex.ready` can fire. * * @private + * @param {Promise} [startupDeadline] * @returns {Promise} */ - async _finalizeReady() { + async _finalizeReady(startupDeadline) { const {credentials} = this.webex; if (credentials && credentials.isRefreshing) { - await new Promise((resolve) => { - credentials.once('change:isRefreshing', resolve); + let onChangeIsRefreshing = () => {}; + const refreshSettled = new Promise((resolve) => { + onChangeIsRefreshing = resolve; + credentials.once('change:isRefreshing', onChangeIsRefreshing); + + if (!credentials.isRefreshing) { + resolve(); + } }); + + await (startupDeadline ? Promise.race([refreshSettled, startupDeadline]) : refreshSettled); + credentials.off('change:isRefreshing', onChangeIsRefreshing); } this.ready = true; @@ -1517,26 +1523,26 @@ const Services = WebexPlugin.extend({ // catalogs. We listen for 'loaded' instead of 'ready' because `services.ready` // now blocks `webex.ready` - listening to 'ready' would deadlock. this.listenToOnce(this.webex, 'loaded', async () => { + const initTimeoutMs = this.webex.config?.services?.catalogInitTimeout; + let cancelStartupDeadline = () => {}; + const startupDeadline = new Promise((resolve) => { + const timeout = setTimeout(resolve, initTimeoutMs); + + cancelStartupDeadline = () => clearTimeout(timeout); + }); + const finalizeReady = () => + this._finalizeReady(startupDeadline).finally(cancelStartupDeadline); + const cachedCatalog = await this._loadCatalogFromCache(); if (cachedCatalog) { catalog.isReady = true; - await this._finalizeReady(); + await finalizeReady(); return; // skip initServiceCatalogs() on reload when cache exists } const {supertoken} = this.webex.credentials; - - // Race init against a hard timeout so a hung request never leaves - // `services.ready` false forever - that would stall `webex.ready` and - // leave consumers waiting on it indefinitely. Timeout is configurable via - // `config.services.catalogInitTimeout` (defaults to 15s in config). - const initTimeoutMs = this.webex.config?.services?.catalogInitTimeout; - - const initServiceCatalogsTimeout = new Promise((_, reject) => { - setTimeout( - () => reject(new Error(`services: init timed out after ${initTimeoutMs}ms`)), - initTimeoutMs - ); + const initServiceCatalogsTimeout = startupDeadline.then(() => { + throw new Error(`services: init timed out after ${initTimeoutMs}ms`); }); // Validate if the supertoken exists. @@ -1551,7 +1557,7 @@ const Services = WebexPlugin.extend({ `services: failed to init initial services when credentials available, ${error?.message}` ); }) - .finally(() => this._finalizeReady()); + .finally(finalizeReady); } else { const {email} = this.webex.config; @@ -1565,7 +1571,7 @@ const Services = WebexPlugin.extend({ `services: failed to init initial services when no credentials available, ${error?.message}` ); }) - .finally(() => this._finalizeReady()); + .finally(finalizeReady); // Handle fresh login: 'loaded' fires before OAuth completes, so listen // for `canAuthorize` flipping true and then collect the postauth catalog. diff --git a/packages/@webex/webex-core/test/unit/spec/services-v2/services-v2.ts b/packages/@webex/webex-core/test/unit/spec/services-v2/services-v2.ts index 1e6ba3fd7e3..a8327bc0926 100644 --- a/packages/@webex/webex-core/test/unit/spec/services-v2/services-v2.ts +++ b/packages/@webex/webex-core/test/unit/spec/services-v2/services-v2.ts @@ -266,6 +266,41 @@ describe('webex-core', () => { ); }); + it('does not extend the init timeout for an in-flight credentials refresh', async () => { + const clock = sinon.useFakeTimers(); + let onChangeIsRefreshing: (() => void) | undefined; + + services.listenToOnce = sinon.stub(); + services.initServiceCatalogs = sinon.stub().returns(new Promise(() => {})); + services.webex.credentials = { + supertoken: {access_token: 'token'}, + isRefreshing: true, + once: sinon.stub().callsFake((event: string, callback: () => void) => { + if (event === 'change:isRefreshing') onChangeIsRefreshing = callback; + }), + off: sinon.stub(), + }; + services.logger.error = sinon.stub(); + + services.initialize(); + services.listenToOnce.getCall(0).args[2](); + services.listenToOnce.getCall(1).args[2](); + + await clock.tickAsync(15_000); + clock.restore(); + + assert.isTrue(services.initFailed); + assert.isTrue( + services.ready, + 'credential refresh must not extend the catalog init deadline' + ); + sinon.assert.calledOnceWithExactly( + services.webex.credentials.off, + 'change:isRefreshing', + onChangeIsRefreshing + ); + }); + it('awaits an in-flight credentials refresh before flipping services.ready=true', async () => { services.listenToOnce = sinon.stub(); services.initServiceCatalogs = sinon.stub().returns(Promise.resolve()); @@ -277,6 +312,7 @@ describe('webex-core', () => { once: sinon.stub().callsFake((event: string, cb: () => void) => { if (event === 'change:isRefreshing') onChangeIsRefreshing = cb; }), + off: sinon.stub(), }; services.initialize(); @@ -407,6 +443,7 @@ describe('webex-core', () => { once: sinon.stub().callsFake((event: string, cb: () => void) => { if (event === 'change:isRefreshing') onChangeIsRefreshing = cb; }), + off: sinon.stub(), }; const settled = services._finalizeReady(); @@ -420,6 +457,37 @@ describe('webex-core', () => { assert.isTrue(services.ready); }); + + it('sets ready=true when the startup deadline settles before credentials refresh', async () => { + let resolveDeadline = () => {}; + let onChangeIsRefreshing: (() => void) | undefined; + const deadline = new Promise((resolve) => { + resolveDeadline = resolve; + }); + + services.webex.credentials = { + isRefreshing: true, + once: sinon.stub().callsFake((event: string, callback: () => void) => { + if (event === 'change:isRefreshing') onChangeIsRefreshing = callback; + }), + off: sinon.stub(), + }; + + const settled = services._finalizeReady(deadline); + + await waitForAsync(); + assert.isFalse(services.ready); + + resolveDeadline(); + await settled; + + assert.isTrue(services.ready); + sinon.assert.calledOnceWithExactly( + services.webex.credentials.off, + 'change:isRefreshing', + onChangeIsRefreshing + ); + }); }); describe('#initServiceCatalogs', () => { diff --git a/packages/@webex/webex-core/test/unit/spec/services/services.js b/packages/@webex/webex-core/test/unit/spec/services/services.js index be88c4c5954..5f805f99a8e 100644 --- a/packages/@webex/webex-core/test/unit/spec/services/services.js +++ b/packages/@webex/webex-core/test/unit/spec/services/services.js @@ -269,6 +269,41 @@ describe('webex-core', () => { ); }); + it('does not extend the init timeout for an in-flight credentials refresh', async () => { + const clock = sinon.useFakeTimers(); + let onChangeIsRefreshing; + + services.listenToOnce = sinon.stub(); + services.initServiceCatalogs = sinon.stub().returns(new Promise(() => {})); + services.webex.credentials = { + supertoken: {access_token: 'token'}, + isRefreshing: true, + once: sinon.stub().callsFake((event, callback) => { + if (event === 'change:isRefreshing') onChangeIsRefreshing = callback; + }), + off: sinon.stub(), + }; + services.logger.error = sinon.stub(); + + services.initialize(); + services.listenToOnce.getCall(0).args[2](); + services.listenToOnce.getCall(1).args[2](); + + await clock.tickAsync(15_000); + clock.restore(); + + assert.isTrue(services.initFailed); + assert.isTrue( + services.ready, + 'credential refresh must not extend the catalog init deadline' + ); + sinon.assert.calledOnceWithExactly( + services.webex.credentials.off, + 'change:isRefreshing', + onChangeIsRefreshing + ); + }); + it('awaits an in-flight credentials refresh before flipping services.ready=true', async () => { services.listenToOnce = sinon.stub(); services.initServiceCatalogs = sinon.stub().returns(Promise.resolve()); @@ -280,6 +315,7 @@ describe('webex-core', () => { once: sinon.stub().callsFake((event, cb) => { if (event === 'change:isRefreshing') onChangeIsRefreshing = cb; }), + off: sinon.stub(), }; services.initialize(); @@ -413,6 +449,7 @@ describe('webex-core', () => { once: sinon.stub().callsFake((event, cb) => { if (event === 'change:isRefreshing') onChangeIsRefreshing = cb; }), + off: sinon.stub(), }; const settled = services._finalizeReady(); @@ -427,6 +464,37 @@ describe('webex-core', () => { assert.isTrue(services.ready); }); + + it('sets ready=true when the startup deadline settles before credentials refresh', async () => { + let resolveDeadline = () => {}; + let onChangeIsRefreshing; + const deadline = new Promise((resolve) => { + resolveDeadline = resolve; + }); + + services.webex.credentials = { + isRefreshing: true, + once: sinon.stub().callsFake((event, callback) => { + if (event === 'change:isRefreshing') onChangeIsRefreshing = callback; + }), + off: sinon.stub(), + }; + + const settled = services._finalizeReady(deadline); + + await waitForAsync(); + assert.isFalse(services.ready); + + resolveDeadline(); + await settled; + + assert.isTrue(services.ready); + sinon.assert.calledOnceWithExactly( + services.webex.credentials.off, + 'change:isRefreshing', + onChangeIsRefreshing + ); + }); }); describe('#initServiceCatalogs', () => {