Skip to content

Commit 45a66e5

Browse files
cboldisclaude
cboldis
and
claude
committed
fix(llmo): address MysticatBot review comments on URL Inspector Semrush integration
- Extract hostname from full URL for CBF_domain eq filter (domain != full URL) - Add per-URL error isolation in owned-urls fan-out so one failure preserves other rows - Re-throw ErrorWithStatusCode(503) config errors instead of falling back silently - Add tests: 503 re-throw, per-URL isolation, hostname extraction verification Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 4e7c47a commit 45a66e5

2 files changed

Lines changed: 140 additions & 16 deletions

File tree

‎src/controllers/llmo/llmo-url-inspector.js‎

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -214,6 +214,12 @@ export function createUrlInspectorStatsHandler(getOrgAndValidateAccess) {
214214
// non-citation KPIs (totalPrompts, uniqueUrls) stay from Mysticat.
215215
const semrushWorkspaceId = await resolveWorkspaceId(ctx, spaceCatId);
216216
if (semrushWorkspaceId && filterByBrandId) {
217+
let siteDomain;
218+
try {
219+
siteDomain = new URL(params.siteId).hostname;
220+
} catch {
221+
siteDomain = params.siteId;
222+
}
217223
try {
218224
const imsToken = ctx?.pathInfo?.headers?.authorization?.replace(/^Bearer\s+/i, '');
219225
const transport = createSerenityTransport({ env: ctx.env, imsToken });
@@ -225,7 +231,7 @@ export function createUrlInspectorStatsHandler(getOrgAndValidateAccess) {
225231
ctx.env,
226232
{
227233
urlFragment: params.siteId,
228-
domain: params.siteId,
234+
domain: siteDomain,
229235
startDate: rpcParams.p_start_date,
230236
endDate: rpcParams.p_end_date,
231237
},
@@ -235,6 +241,9 @@ export function createUrlInspectorStatsHandler(getOrgAndValidateAccess) {
235241
stats.totalPromptsCited = bpResult.promptsCited;
236242
}
237243
} catch (e) {
244+
if (e?.status === 503) {
245+
throw e;
246+
}
238247
ctx.log.error(`URL Inspector stats Semrush BP error: ${e?.message || e}`);
239248
// Fall through: return Mysticat data rather than surfacing a 500
240249
}
@@ -364,11 +373,17 @@ export function createUrlInspectorOwnedUrlsHandler(getOrgAndValidateAccess) {
364373
// per URL with data from the BP reporting API.
365374
const semrushWorkspaceId = await resolveWorkspaceId(ctx, spaceCatId);
366375
if (semrushWorkspaceId && filterByBrandId && urlsFromMysticat.length > 0) {
367-
try {
368-
const imsToken = ctx?.pathInfo?.headers?.authorization?.replace(/^Bearer\s+/i, '');
369-
const transport = createSerenityTransport({ env: ctx.env, imsToken });
370-
const urlsWithSemrush = await Promise.all(
371-
urlsFromMysticat.map(async (urlRow) => {
376+
const imsToken = ctx?.pathInfo?.headers?.authorization?.replace(/^Bearer\s+/i, '');
377+
const transport = createSerenityTransport({ env: ctx.env, imsToken });
378+
urlsFromMysticat = await Promise.all(
379+
urlsFromMysticat.map(async (urlRow) => {
380+
let urlDomain;
381+
try {
382+
urlDomain = new URL(urlRow.url).hostname;
383+
} catch {
384+
urlDomain = urlRow.url;
385+
}
386+
try {
372387
const bpResult = await queryBpCitationsByUrl(
373388
transport,
374389
ctx.dataAccess,
@@ -377,7 +392,7 @@ export function createUrlInspectorOwnedUrlsHandler(getOrgAndValidateAccess) {
377392
ctx.env,
378393
{
379394
urlFragment: urlRow.url,
380-
domain: urlRow.url,
395+
domain: urlDomain,
381396
startDate: rpcParams.p_start_date,
382397
endDate: rpcParams.p_end_date,
383398
},
@@ -390,13 +405,15 @@ export function createUrlInspectorOwnedUrlsHandler(getOrgAndValidateAccess) {
390405
citations: bpResult.citations,
391406
promptsCited: bpResult.promptsCited,
392407
};
393-
}),
394-
);
395-
urlsFromMysticat = urlsWithSemrush;
396-
} catch (e) {
397-
ctx.log.error(`URL Inspector owned URLs Semrush BP error: ${e?.message || e}`);
398-
// Fall through: return Mysticat data rather than surfacing a 500
399-
}
408+
} catch (e) {
409+
if (e?.status === 503) {
410+
throw e;
411+
}
412+
ctx.log.warn(`URL Inspector owned URLs Semrush BP error for ${urlRow.url}: ${e?.message || e}`);
413+
return urlRow;
414+
}
415+
}),
416+
);
400417
}
401418

402419
return cachedOk({ urls: urlsFromMysticat, totalCount });
@@ -709,6 +726,12 @@ export function createUrlInspectorUrlPromptsHandler(
709726
try {
710727
const imsToken = ctx?.pathInfo?.headers?.authorization?.replace(/^Bearer\s+/i, '');
711728
const transport = createSerenityTransport({ env: ctx.env, imsToken });
729+
let urlDomain;
730+
try {
731+
urlDomain = new URL(urlId).hostname;
732+
} catch {
733+
urlDomain = urlId;
734+
}
712735
const bpResult = await queryBpCitationsByUrl(
713736
transport,
714737
ctx.dataAccess,
@@ -717,7 +740,7 @@ export function createUrlInspectorUrlPromptsHandler(
717740
ctx.env,
718741
{
719742
urlFragment: urlId,
720-
domain: urlId,
743+
domain: urlDomain,
721744
startDate,
722745
endDate,
723746
},
@@ -726,6 +749,9 @@ export function createUrlInspectorUrlPromptsHandler(
726749
return cachedOk({ prompts: bpResult.prompts });
727750
}
728751
} catch (e) {
752+
if (e?.status === 503) {
753+
throw e;
754+
}
729755
ctx.log.error(`URL Inspector URL prompts Semrush BP error: ${e?.message || e}`);
730756
// Fall through to Mysticat path
731757
}

‎test/controllers/llmo/llmo-url-inspector.test.js‎

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2193,6 +2193,32 @@ describe('URL Inspector Semrush BP integration', () => {
21932193
expect(response.status).to.equal(200);
21942194
expect(body.stats.totalCitations).to.equal(77);
21952195
});
2196+
2197+
it('re-throws 503 config errors without falling back', async () => {
2198+
const configError = Object.assign(new Error('missing element id'), { status: 503 });
2199+
queryBpCitationsByUrlStub.rejects(configError);
2200+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2201+
rpcStub.resolves({ data: [{ week: null, value: 10 }], error: null });
2202+
2203+
const handler = Handlers.createUrlInspectorStatsHandler(
2204+
async () => ({ organization: { getId: () => ORG_ID } }),
2205+
);
2206+
await expect(handler(context)).to.be.rejectedWith('missing element id');
2207+
});
2208+
2209+
it('passes hostname (not full URL) as domain to queryBpCitationsByUrl', async () => {
2210+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID }, { siteId: 'https://www.example.com' });
2211+
rpcStub.resolves({ data: [{ week: null, value: 0 }], error: null });
2212+
2213+
const handler = Handlers.createUrlInspectorStatsHandler(
2214+
async () => ({ organization: { getId: () => ORG_ID } }),
2215+
);
2216+
await handler(context);
2217+
2218+
const callArgs = queryBpCitationsByUrlStub.firstCall.args[5];
2219+
expect(callArgs.domain).to.equal('www.example.com');
2220+
expect(callArgs.urlFragment).to.equal('https://www.example.com');
2221+
});
21962222
});
21972223

21982224
describe('createUrlInspectorOwnedUrlsHandler — Semrush path', () => {
@@ -2276,7 +2302,54 @@ describe('URL Inspector Semrush BP integration', () => {
22762302

22772303
expect(response.status).to.equal(200);
22782304
expect(body.urls[0].citations).to.equal(5);
2279-
expect(context.log.error).to.have.been.calledWithMatch(/Semrush BP error/);
2305+
expect(context.log.warn).to.have.been.calledWithMatch(/Semrush BP error/);
2306+
});
2307+
2308+
it('preserves successful rows when one URL Semrush call fails', async () => {
2309+
queryBpCitationsByUrlStub.onFirstCall().resolves({
2310+
citations: 99, promptsCited: 3, prompts: [],
2311+
});
2312+
queryBpCitationsByUrlStub.onSecondCall().rejects(new Error('partial fail'));
2313+
const row1 = { ...ownedRow(), url: 'https://example.com/page1', citations: 1 };
2314+
const row2 = { ...ownedRow(), url: 'https://example.com/page2', citations: 2 };
2315+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2316+
rpcStub.resolves({ data: [row1, row2], error: null });
2317+
2318+
const handler = Handlers.createUrlInspectorOwnedUrlsHandler(
2319+
async () => ({ organization: { getId: () => ORG_ID } }),
2320+
);
2321+
const response = await handler(context);
2322+
const body = await response.json();
2323+
2324+
expect(response.status).to.equal(200);
2325+
expect(body.urls[0].citations).to.equal(99);
2326+
expect(body.urls[1].citations).to.equal(2);
2327+
});
2328+
2329+
it('re-throws 503 config errors in owned-urls', async () => {
2330+
const configError = Object.assign(new Error('missing element id'), { status: 503 });
2331+
queryBpCitationsByUrlStub.rejects(configError);
2332+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2333+
rpcStub.resolves({ data: [ownedRow()], error: null });
2334+
2335+
const handler = Handlers.createUrlInspectorOwnedUrlsHandler(
2336+
async () => ({ organization: { getId: () => ORG_ID } }),
2337+
);
2338+
await expect(handler(context)).to.be.rejectedWith('missing element id');
2339+
});
2340+
2341+
it('passes hostname (not full URL) as domain in per-URL query', async () => {
2342+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2343+
rpcStub.resolves({ data: [ownedRow()], error: null });
2344+
2345+
const handler = Handlers.createUrlInspectorOwnedUrlsHandler(
2346+
async () => ({ organization: { getId: () => ORG_ID } }),
2347+
);
2348+
await handler(context);
2349+
2350+
const callArgs = queryBpCitationsByUrlStub.firstCall.args[5];
2351+
expect(callArgs.domain).to.equal('example.com');
2352+
expect(callArgs.urlFragment).to.equal('https://example.com/page');
22802353
});
22812354
});
22822355

@@ -2340,5 +2413,30 @@ describe('URL Inspector Semrush BP integration', () => {
23402413
expect(body.prompts[0].prompt).to.equal('fallback prompt');
23412414
expect(context.log.error).to.have.been.calledWithMatch(/Semrush BP error/);
23422415
});
2416+
2417+
it('re-throws 503 config errors in url-prompts', async () => {
2418+
const configError = Object.assign(new Error('missing element id'), { status: 503 });
2419+
queryBpCitationsByUrlStub.rejects(configError);
2420+
const { context } = createSemrushContext({ brandId: BRAND_ID });
2421+
2422+
const handler = Handlers.createUrlInspectorUrlPromptsHandler(
2423+
async () => ({ organization: { getId: () => ORG_ID } }),
2424+
);
2425+
const ctxWithUrl = { ...context, data: { siteId: SITE_ID, urlId: 'https://example.com/page' } };
2426+
await expect(handler(ctxWithUrl)).to.be.rejectedWith('missing element id');
2427+
});
2428+
2429+
it('passes hostname as domain for url-prompts when urlId is a URL', async () => {
2430+
const { context } = createSemrushContext({ brandId: BRAND_ID });
2431+
2432+
const handler = Handlers.createUrlInspectorUrlPromptsHandler(
2433+
async () => ({ organization: { getId: () => ORG_ID } }),
2434+
);
2435+
const ctxWithUrl = { ...context, data: { siteId: SITE_ID, urlId: 'https://example.com/page' } };
2436+
await handler(ctxWithUrl);
2437+
2438+
const callArgs = queryBpCitationsByUrlStub.firstCall.args[5];
2439+
expect(callArgs.domain).to.equal('example.com');
2440+
});
23432441
});
23442442
});

0 commit comments

Comments
 (0)