Skip to content

fix(webex-core): bound services readiness by startup deadline #5140

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

Open
robstax wants to merge 1 commit into
base: next
Choose a base branch
from
Open

fix(webex-core): bound services readiness by startup deadline #5140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
53 changes: 30 additions & 23 deletions packages/@webex/webex-core/src/lib/services-v2/services-v2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>} [startupDeadline]
* @returns {Promise<void>}
*/
async _finalizeReady(): Promise<void> {
async _finalizeReady(startupDeadline?: Promise<void>): Promise<void> {
const {credentials} = this.webex;

if (credentials && credentials.isRefreshing) {
await new Promise<void>((resolve) => {
credentials.once('change:isRefreshing', resolve);
let onChangeIsRefreshing = () => {};
const refreshSettled = new Promise<void>((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;
Expand Down Expand Up @@ -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<void>((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<never>((_, 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.
Expand All @@ -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;

Expand All @@ -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.
Expand Down
54 changes: 30 additions & 24 deletions packages/@webex/webex-core/src/lib/services/services.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<void>} [startupDeadline]
* @returns {Promise<void>}
*/
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;
Expand Down Expand Up @@ -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.
Expand All @@ -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;

Expand All @@ -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.
Expand Down
68 changes: 68 additions & 0 deletions packages/@webex/webex-core/test/unit/spec/services-v2/services-v2.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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<void>((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', () => {
Expand Down
68 changes: 68 additions & 0 deletions packages/@webex/webex-core/test/unit/spec/services/services.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand All @@ -280,6 +315,7 @@ describe('webex-core', () => {
once: sinon.stub().callsFake((event, cb) => {
if (event === 'change:isRefreshing') onChangeIsRefreshing = cb;
}),
off: sinon.stub(),
};

services.initialize();
Expand Down Expand Up @@ -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();
Expand All @@ -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', () => {
Expand Down
Loading