Skip to content

Commit ae763eb

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 f3a9c00 commit ae763eb

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
}
@@ -371,11 +380,17 @@ export function createUrlInspectorOwnedUrlsHandler(getOrgAndValidateAccess) {
371380
// per URL with data from the BP reporting API.
372381
const semrushWorkspaceId = await resolveWorkspaceId(ctx, spaceCatId);
373382
if (semrushWorkspaceId && filterByBrandId && urlsFromMysticat.length > 0) {
374-
try {
375-
const imsToken = ctx?.pathInfo?.headers?.authorization?.replace(/^Bearer\s+/i, '');
376-
const transport = createSerenityTransport({ env: ctx.env, imsToken });
377-
const urlsWithSemrush = await Promise.all(
378-
urlsFromMysticat.map(async (urlRow) => {
383+
const imsToken = ctx?.pathInfo?.headers?.authorization?.replace(/^Bearer\s+/i, '');
384+
const transport = createSerenityTransport({ env: ctx.env, imsToken });
385+
urlsFromMysticat = await Promise.all(
386+
urlsFromMysticat.map(async (urlRow) => {
387+
let urlDomain;
388+
try {
389+
urlDomain = new URL(urlRow.url).hostname;
390+
} catch {
391+
urlDomain = urlRow.url;
392+
}
393+
try {
379394
const bpResult = await queryBpCitationsByUrl(
380395
transport,
381396
ctx.dataAccess,
@@ -384,7 +399,7 @@ export function createUrlInspectorOwnedUrlsHandler(getOrgAndValidateAccess) {
384399
ctx.env,
385400
{
386401
urlFragment: urlRow.url,
387-
domain: urlRow.url,
402+
domain: urlDomain,
388403
startDate: rpcParams.p_start_date,
389404
endDate: rpcParams.p_end_date,
390405
},
@@ -397,13 +412,15 @@ export function createUrlInspectorOwnedUrlsHandler(getOrgAndValidateAccess) {
397412
citations: bpResult.citations,
398413
promptsCited: bpResult.promptsCited,
399414
};
400-
}),
401-
);
402-
urlsFromMysticat = urlsWithSemrush;
403-
} catch (e) {
404-
ctx.log.error(`URL Inspector owned URLs Semrush BP error: ${e?.message || e}`);
405-
// Fall through: return Mysticat data rather than surfacing a 500
406-
}
415+
} catch (e) {
416+
if (e?.status === 503) {
417+
throw e;
418+
}
419+
ctx.log.warn(`URL Inspector owned URLs Semrush BP error for ${urlRow.url}: ${e?.message || e}`);
420+
return urlRow;
421+
}
422+
}),
423+
);
407424
}
408425

409426
return cachedOk({ urls: urlsFromMysticat, totalCount });
@@ -716,6 +733,12 @@ export function createUrlInspectorUrlPromptsHandler(
716733
try {
717734
const imsToken = ctx?.pathInfo?.headers?.authorization?.replace(/^Bearer\s+/i, '');
718735
const transport = createSerenityTransport({ env: ctx.env, imsToken });
736+
let urlDomain;
737+
try {
738+
urlDomain = new URL(urlId).hostname;
739+
} catch {
740+
urlDomain = urlId;
741+
}
719742
const bpResult = await queryBpCitationsByUrl(
720743
transport,
721744
ctx.dataAccess,
@@ -724,7 +747,7 @@ export function createUrlInspectorUrlPromptsHandler(
724747
ctx.env,
725748
{
726749
urlFragment: urlId,
727-
domain: urlId,
750+
domain: urlDomain,
728751
startDate,
729752
endDate,
730753
},
@@ -733,6 +756,9 @@ export function createUrlInspectorUrlPromptsHandler(
733756
return cachedOk({ prompts: bpResult.prompts });
734757
}
735758
} catch (e) {
759+
if (e?.status === 503) {
760+
throw e;
761+
}
736762
ctx.log.error(`URL Inspector URL prompts Semrush BP error: ${e?.message || e}`);
737763
// Fall through to Mysticat path
738764
}

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

Lines changed: 99 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2223,6 +2223,32 @@ describe('URL Inspector Semrush BP integration', () => {
22232223
expect(response.status).to.equal(200);
22242224
expect(body.stats.totalCitations).to.equal(77);
22252225
});
2226+
2227+
it('re-throws 503 config errors without falling back', async () => {
2228+
const configError = Object.assign(new Error('missing element id'), { status: 503 });
2229+
queryBpCitationsByUrlStub.rejects(configError);
2230+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2231+
rpcStub.resolves({ data: [{ week: null, value: 10 }], error: null });
2232+
2233+
const handler = Handlers.createUrlInspectorStatsHandler(
2234+
async () => ({ organization: { getId: () => ORG_ID } }),
2235+
);
2236+
await expect(handler(context)).to.be.rejectedWith('missing element id');
2237+
});
2238+
2239+
it('passes hostname (not full URL) as domain to queryBpCitationsByUrl', async () => {
2240+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID }, { siteId: 'https://www.example.com' });
2241+
rpcStub.resolves({ data: [{ week: null, value: 0 }], error: null });
2242+
2243+
const handler = Handlers.createUrlInspectorStatsHandler(
2244+
async () => ({ organization: { getId: () => ORG_ID } }),
2245+
);
2246+
await handler(context);
2247+
2248+
const callArgs = queryBpCitationsByUrlStub.firstCall.args[5];
2249+
expect(callArgs.domain).to.equal('www.example.com');
2250+
expect(callArgs.urlFragment).to.equal('https://www.example.com');
2251+
});
22262252
});
22272253

22282254
describe('createUrlInspectorOwnedUrlsHandler — Semrush path', () => {
@@ -2306,7 +2332,54 @@ describe('URL Inspector Semrush BP integration', () => {
23062332

23072333
expect(response.status).to.equal(200);
23082334
expect(body.urls[0].citations).to.equal(5);
2309-
expect(context.log.error).to.have.been.calledWithMatch(/Semrush BP error/);
2335+
expect(context.log.warn).to.have.been.calledWithMatch(/Semrush BP error/);
2336+
});
2337+
2338+
it('preserves successful rows when one URL Semrush call fails', async () => {
2339+
queryBpCitationsByUrlStub.onFirstCall().resolves({
2340+
citations: 99, promptsCited: 3, prompts: [],
2341+
});
2342+
queryBpCitationsByUrlStub.onSecondCall().rejects(new Error('partial fail'));
2343+
const row1 = { ...ownedRow(), url: 'https://example.com/page1', citations: 1 };
2344+
const row2 = { ...ownedRow(), url: 'https://example.com/page2', citations: 2 };
2345+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2346+
rpcStub.resolves({ data: [row1, row2], error: null });
2347+
2348+
const handler = Handlers.createUrlInspectorOwnedUrlsHandler(
2349+
async () => ({ organization: { getId: () => ORG_ID } }),
2350+
);
2351+
const response = await handler(context);
2352+
const body = await response.json();
2353+
2354+
expect(response.status).to.equal(200);
2355+
expect(body.urls[0].citations).to.equal(99);
2356+
expect(body.urls[1].citations).to.equal(2);
2357+
});
2358+
2359+
it('re-throws 503 config errors in owned-urls', async () => {
2360+
const configError = Object.assign(new Error('missing element id'), { status: 503 });
2361+
queryBpCitationsByUrlStub.rejects(configError);
2362+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2363+
rpcStub.resolves({ data: [ownedRow()], error: null });
2364+
2365+
const handler = Handlers.createUrlInspectorOwnedUrlsHandler(
2366+
async () => ({ organization: { getId: () => ORG_ID } }),
2367+
);
2368+
await expect(handler(context)).to.be.rejectedWith('missing element id');
2369+
});
2370+
2371+
it('passes hostname (not full URL) as domain in per-URL query', async () => {
2372+
const { context, rpcStub } = createSemrushContext({ brandId: BRAND_ID });
2373+
rpcStub.resolves({ data: [ownedRow()], error: null });
2374+
2375+
const handler = Handlers.createUrlInspectorOwnedUrlsHandler(
2376+
async () => ({ organization: { getId: () => ORG_ID } }),
2377+
);
2378+
await handler(context);
2379+
2380+
const callArgs = queryBpCitationsByUrlStub.firstCall.args[5];
2381+
expect(callArgs.domain).to.equal('example.com');
2382+
expect(callArgs.urlFragment).to.equal('https://example.com/page');
23102383
});
23112384
});
23122385

@@ -2370,5 +2443,30 @@ describe('URL Inspector Semrush BP integration', () => {
23702443
expect(body.prompts[0].prompt).to.equal('fallback prompt');
23712444
expect(context.log.error).to.have.been.calledWithMatch(/Semrush BP error/);
23722445
});
2446+
2447+
it('re-throws 503 config errors in url-prompts', async () => {
2448+
const configError = Object.assign(new Error('missing element id'), { status: 503 });
2449+
queryBpCitationsByUrlStub.rejects(configError);
2450+
const { context } = createSemrushContext({ brandId: BRAND_ID });
2451+
2452+
const handler = Handlers.createUrlInspectorUrlPromptsHandler(
2453+
async () => ({ organization: { getId: () => ORG_ID } }),
2454+
);
2455+
const ctxWithUrl = { ...context, data: { siteId: SITE_ID, urlId: 'https://example.com/page' } };
2456+
await expect(handler(ctxWithUrl)).to.be.rejectedWith('missing element id');
2457+
});
2458+
2459+
it('passes hostname as domain for url-prompts when urlId is a URL', async () => {
2460+
const { context } = createSemrushContext({ brandId: BRAND_ID });
2461+
2462+
const handler = Handlers.createUrlInspectorUrlPromptsHandler(
2463+
async () => ({ organization: { getId: () => ORG_ID } }),
2464+
);
2465+
const ctxWithUrl = { ...context, data: { siteId: SITE_ID, urlId: 'https://example.com/page' } };
2466+
await handler(ctxWithUrl);
2467+
2468+
const callArgs = queryBpCitationsByUrlStub.firstCall.args[5];
2469+
expect(callArgs.domain).to.equal('example.com');
2470+
});
23732471
});
23742472
});

0 commit comments

Comments
 (0)