diff --git a/package-lock.json b/package-lock.json index c2060dff2e..edac5d6926 100644 --- a/package-lock.json +++ b/package-lock.json @@ -27,7 +27,7 @@ "@adobe/spacecat-shared-http-utils": "1.29.1", "@adobe/spacecat-shared-ims-client": "1.12.7", "@adobe/spacecat-shared-launchdarkly-client": "1.3.0", - "@adobe/spacecat-shared-rum-api-client": "2.40.13", + "@adobe/spacecat-shared-rum-api-client": "2.43.1", "@adobe/spacecat-shared-scrape-client": "2.6.3", "@adobe/spacecat-shared-slack-client": "1.6.7", "@adobe/spacecat-shared-tier-client": "1.5.1", @@ -6331,14 +6331,14 @@ } }, "node_modules/@adobe/spacecat-shared-rum-api-client": { - "version": "2.40.13", - "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-rum-api-client/-/spacecat-shared-rum-api-client-2.40.13.tgz", - "integrity": "sha512-UcgbDOvUAmuN32WuTAgwRYxXCfN5C8xyD170KBXRn0pPK4Zlp1xQU3p3nEcZd8+iarDzX4LQsfL+jv6PNTj7xA==", + "version": "2.43.1", + "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-rum-api-client/-/spacecat-shared-rum-api-client-2.43.1.tgz", + "integrity": "sha512-l788awoZkigySsWuM90IrvpH84pNzDmh0av7iyYGHMHuYxkEI9OYZNskm26ff1Oi5VSvjthueNC4c5ndYIjbpA==", "license": "Apache-2.0", "dependencies": { "@adobe/fetch": "4.3.0", "@adobe/helix-shared-wrap": "2.0.2", - "@adobe/helix-universal": "5.4.1", + "@adobe/helix-universal": "5.4.2", "@adobe/rum-distiller": "1.23.0", "@adobe/spacecat-shared-utils": "1.81.1", "aws4": "1.13.2", @@ -6349,6 +6349,16 @@ "npm": ">=10.9.0 <12.0.0" } }, + "node_modules/@adobe/spacecat-shared-rum-api-client/node_modules/@adobe/helix-universal": { + "version": "5.4.2", + "resolved": "https://registry.npmjs.org/@adobe/helix-universal/-/helix-universal-5.4.2.tgz", + "integrity": "sha512-LIF6K7YQ78/Wxw2OQY+f5HxdIsoZnsJCcNWm9fnBQBRGHBIh3hQq+krD1yRWe/pkBVxAm7o4FJs8SmJkJjU0LQ==", + "license": "Apache-2.0", + "dependencies": { + "@adobe/fetch": "4.3.0", + "aws4": "1.13.2" + } + }, "node_modules/@adobe/spacecat-shared-rum-api-client/node_modules/@adobe/spacecat-shared-utils": { "version": "1.81.1", "resolved": "https://registry.npmjs.org/@adobe/spacecat-shared-utils/-/spacecat-shared-utils-1.81.1.tgz", diff --git a/package.json b/package.json index 955b3f8954..c3b3c03715 100644 --- a/package.json +++ b/package.json @@ -91,7 +91,7 @@ "@adobe/spacecat-shared-http-utils": "1.29.1", "@adobe/spacecat-shared-ims-client": "1.12.7", "@adobe/spacecat-shared-launchdarkly-client": "1.3.0", - "@adobe/spacecat-shared-rum-api-client": "2.40.13", + "@adobe/spacecat-shared-rum-api-client": "2.43.1", "@adobe/spacecat-shared-scrape-client": "2.6.3", "@adobe/spacecat-shared-slack-client": "1.6.7", "@adobe/spacecat-shared-tier-client": "1.5.1", diff --git a/src/support/rum-config-service.js b/src/support/rum-config-service.js index 0bdc2fcfb2..855916740f 100644 --- a/src/support/rum-config-service.js +++ b/src/support/rum-config-service.js @@ -10,17 +10,14 @@ * governing permissions and limitations under the License. */ -import RUMAPIClient from '@adobe/spacecat-shared-rum-api-client'; +import { resolveRumDomainKey } from '@adobe/spacecat-shared-rum-api-client'; import { Config } from '@adobe/spacecat-shared-data-access/src/models/site/config.js'; -const RUM_CHECK_TIMEOUT_MS = 3000; - /** * Checks whether the site has a RUM domain key and optionally persists the result. * - * Tries candidates in priority order (override-first, www variants as fallback) and - * stops on the first successful domain key lookup. The 3 s timeout is a shared budget - * across the full candidate loop, not a per-domain limit. + * Candidate resolution and timeout logic live in the shared + * {@link resolveRumDomainKey} helper; this function owns only the save semantics. * * When called with the default { save: true }, this function applies the rumConfig * update and saves the site internally. @@ -41,70 +38,15 @@ const RUM_CHECK_TIMEOUT_MS = 3000; * @returns {Promise} true if a RUM domain key was found. */ export async function updateRumConfig(site, context, { save = true } = {}) { - const { log } = context; - - const siteConfig = site.getConfig(); - const overrideBaseURL = siteConfig.getFetchConfig()?.overrideBaseURL; - - let overrideHostname = null; - if (overrideBaseURL) { - try { - overrideHostname = new URL(overrideBaseURL).hostname; - } catch { - log.warn(`[rum-config-service] Malformed overrideBaseURL for site ${site.getId()}: ${overrideBaseURL}, falling back to baseURL`); - } - } - - const baseHostname = new URL(site.getBaseURL()).hostname; - const withWwwFallback = (d) => (d && !d.startsWith('www.') ? `www.${d}` : null); - - const domains = [...new Set([ - overrideHostname, - withWwwFallback(overrideHostname), - baseHostname, - withWwwFallback(baseHostname), - ].filter(Boolean))]; - let hasDomainKey = false; - let timeoutId; - let cancelled = false; - - const rumApiClient = RUMAPIClient.createFrom(context); - try { - await Promise.race([ - (async () => { - for (const domain of domains) { - if (cancelled) { - break; - } - try { - // eslint-disable-next-line no-await-in-loop - await rumApiClient.retrieveDomainkey(domain); - hasDomainKey = true; - return; - } catch (e) { - log.info(`[rum-config-service] RUM check failed for ${domain}: ${e.message}`); - } - } - if (!hasDomainKey) { - log.warn(`[rum-config-service] No domain key found across all candidates: ${domains.join(', ')}`); - } - })(), - new Promise((_, reject) => { - timeoutId = setTimeout(() => { - cancelled = true; - reject(new Error('RUM check timed out')); - }, RUM_CHECK_TIMEOUT_MS); - }), - ]); + ({ hasDomainKey } = await resolveRumDomainKey(site, context)); } catch (e) { - log.warn(`[rum-config-service] RUM check failed: ${e.message}`); - } finally { - clearTimeout(timeoutId); + context.log.warn(`[rum-config-service] resolveRumDomainKey failed: ${e.message}`); } if (save) { + const siteConfig = site.getConfig(); siteConfig.updateRumConfig(hasDomainKey); site.setConfig(Config.toDynamoItem(siteConfig)); await site.save(); diff --git a/test/support/rum-config-service.test.js b/test/support/rum-config-service.test.js index 7b771e32e2..0495b6abc0 100644 --- a/test/support/rum-config-service.test.js +++ b/test/support/rum-config-service.test.js @@ -21,21 +21,19 @@ describe('rum-config-service', () => { const sandbox = sinon.createSandbox(); let updateRumConfig; - let retrieveDomainkeyStub; - let rumApiClientStub; + let resolveRumDomainKeyStub; let toDynamoItemStub; let site; let siteConfig; let context; before(async () => { - retrieveDomainkeyStub = sandbox.stub(); - rumApiClientStub = { retrieveDomainkey: retrieveDomainkeyStub }; + resolveRumDomainKeyStub = sandbox.stub(); toDynamoItemStub = sandbox.stub().returns({}); ({ updateRumConfig } = await esmock('../../src/support/rum-config-service.js', { '@adobe/spacecat-shared-rum-api-client': { - default: { createFrom: () => rumApiClientStub }, + resolveRumDomainKey: resolveRumDomainKeyStub, }, '@adobe/spacecat-shared-data-access/src/models/site/config.js': { Config: { toDynamoItem: toDynamoItemStub }, @@ -47,7 +45,6 @@ describe('rum-config-service', () => { sandbox.reset(); siteConfig = { - getFetchConfig: sandbox.stub().returns({}), updateRumConfig: sandbox.stub(), }; @@ -63,60 +60,54 @@ describe('rum-config-service', () => { }); describe('updateRumConfig', () => { - it('sets hasDomainKey true and saves when RUM key is found on base hostname', async () => { - retrieveDomainkeyStub.resolves('dom-key-abc'); + it('returns true and saves when resolveRumDomainKey finds a key', async () => { + resolveRumDomainKeyStub.resolves({ hasDomainKey: true, timedOut: false }); const result = await updateRumConfig(site, context); expect(result).to.be.true; + expect(resolveRumDomainKeyStub).to.have.been.calledOnceWith(site, context); expect(siteConfig.updateRumConfig).to.have.been.calledOnceWith(true); expect(toDynamoItemStub).to.have.been.calledOnceWith(siteConfig); expect(site.setConfig).to.have.been.calledOnce; expect(site.save).to.have.been.calledOnce; }); - it('falls back to www.example.com when example.com lookup fails', async () => { - retrieveDomainkeyStub - .withArgs('example.com').rejects(new Error('not found')) - .withArgs('www.example.com').resolves('dom-key-www'); + it('returns false and saves when resolveRumDomainKey finds no key', async () => { + resolveRumDomainKeyStub.resolves({ hasDomainKey: false, timedOut: false }); const result = await updateRumConfig(site, context); - expect(result).to.be.true; - expect(retrieveDomainkeyStub).to.have.been.calledWith('example.com'); - expect(retrieveDomainkeyStub).to.have.been.calledWith('www.example.com'); - expect(siteConfig.updateRumConfig).to.have.been.calledOnceWith(true); + expect(result).to.be.false; + expect(siteConfig.updateRumConfig).to.have.been.calledOnceWith(false); + expect(toDynamoItemStub).to.have.been.calledOnceWith(siteConfig); + expect(site.setConfig).to.have.been.calledOnce; + expect(site.save).to.have.been.calledOnce; }); - it('sets hasDomainKey false and saves when all candidates fail', async () => { - retrieveDomainkeyStub.rejects(new Error('not found')); + it('returns false and saves when resolveRumDomainKey times out', async () => { + resolveRumDomainKeyStub.resolves({ hasDomainKey: false, timedOut: true }); const result = await updateRumConfig(site, context); expect(result).to.be.false; expect(siteConfig.updateRumConfig).to.have.been.calledOnceWith(false); - expect(toDynamoItemStub).to.have.been.calledOnceWith(siteConfig); - expect(site.setConfig).to.have.been.calledOnce; expect(site.save).to.have.been.calledOnce; }); - it('sets hasDomainKey false and clears timer when RUM check times out', async () => { - retrieveDomainkeyStub.returns(new Promise(() => {})); // never resolves + it('returns false and saves when resolveRumDomainKey throws unexpectedly', async () => { + resolveRumDomainKeyStub.rejects(new Error('unexpected internal error')); - const clock = sinon.useFakeTimers(); - const promise = updateRumConfig(site, context); - await clock.tickAsync(4000); - const result = await promise; - clock.restore(); + const result = await updateRumConfig(site, context); expect(result).to.be.false; + expect(context.log.warn).to.have.been.calledWithMatch(/resolveRumDomainKey failed/); expect(siteConfig.updateRumConfig).to.have.been.calledOnceWith(false); - expect(site.setConfig).to.have.been.calledOnce; expect(site.save).to.have.been.calledOnce; }); it('skips config mutation and save when { save: false } is passed', async () => { - retrieveDomainkeyStub.resolves('dom-key-abc'); + resolveRumDomainKeyStub.resolves({ hasDomainKey: true, timedOut: false }); const result = await updateRumConfig(site, context, { save: false }); @@ -125,95 +116,5 @@ describe('rum-config-service', () => { expect(site.setConfig).to.not.have.been.called; expect(site.save).to.not.have.been.called; }); - - it('tries overrideBaseURL hostname first when set', async () => { - siteConfig.getFetchConfig.returns({ overrideBaseURL: 'https://override.example.com' }); - retrieveDomainkeyStub - .withArgs('override.example.com').resolves('dom-key-override'); - - const result = await updateRumConfig(site, context); - - expect(result).to.be.true; - expect(retrieveDomainkeyStub.firstCall.args[0]).to.equal('override.example.com'); - expect(retrieveDomainkeyStub).to.have.been.calledOnce; - }); - - it('falls back through override www, base, and base www when override bare fails', async () => { - siteConfig.getFetchConfig.returns({ overrideBaseURL: 'https://override.example.com' }); - retrieveDomainkeyStub.withArgs('override.example.com') - .rejects(new Error('not found')); - retrieveDomainkeyStub.withArgs('www.override.example.com') - .rejects(new Error('not found')); - retrieveDomainkeyStub.withArgs('example.com') - .rejects(new Error('not found')); - retrieveDomainkeyStub.withArgs('www.example.com') - .resolves('dom-key-www'); - - const result = await updateRumConfig(site, context); - - expect(result).to.be.true; - expect(retrieveDomainkeyStub.args.map((a) => a[0])).to.deep.equal([ - 'override.example.com', - 'www.override.example.com', - 'example.com', - 'www.example.com', - ]); - }); - - it('does not duplicate www candidate when overrideBaseURL already has www', async () => { - siteConfig.getFetchConfig.returns({ overrideBaseURL: 'https://www.override.example.com' }); - retrieveDomainkeyStub.resolves('dom-key-www'); - - await updateRumConfig(site, context); - - const calledDomains = retrieveDomainkeyStub.args.map((a) => a[0]); - expect(calledDomains.filter((d) => d === 'www.override.example.com')).to.have.lengthOf(1); - }); - - it('falls back to baseURL when overrideBaseURL is malformed', async () => { - siteConfig.getFetchConfig.returns({ overrideBaseURL: 'not-a-valid-url' }); - retrieveDomainkeyStub.resolves('dom-key-abc'); - - const result = await updateRumConfig(site, context); - - expect(result).to.be.true; - expect(context.log.warn).to.have.been.calledWithMatch(/Malformed overrideBaseURL/); - expect(retrieveDomainkeyStub.firstCall.args[0]).to.equal('example.com'); - }); - - it('does not add bare variant when baseURL already starts with www', async () => { - site = { ...site, getBaseURL: () => 'https://www.example.com' }; - retrieveDomainkeyStub.resolves('dom-key-www'); - - const result = await updateRumConfig(site, context); - - expect(result).to.be.true; - const calledDomains = retrieveDomainkeyStub.args.map((a) => a[0]); - expect(calledDomains).to.deep.equal(['www.example.com']); - }); - - it('stops iterating candidates once cancelled by timeout', async () => { - let rejectFirst; - retrieveDomainkeyStub.withArgs('example.com').returns( - new Promise((_, reject) => { rejectFirst = reject; }), - ); - retrieveDomainkeyStub.withArgs('www.example.com').resolves('dom-key-www'); - - const clock = sinon.useFakeTimers(); - const resultPromise = updateRumConfig(site, context); - - await clock.tickAsync(4000); // fires timeout, sets cancelled=true - clock.restore(); - - rejectFirst(new Error('slow network')); - // drain microtasks so inner IIFE processes the rejection and checks cancelled - await new Promise(setImmediate); - - const result = await resultPromise; - - expect(result).to.be.false; - expect(retrieveDomainkeyStub).to.have.been.calledOnce; - expect(retrieveDomainkeyStub).not.to.have.been.calledWith('www.example.com'); - }); }); });