diff --git a/packages/utils/src/internals/sitemap.ts b/packages/utils/src/internals/sitemap.ts index 5587d27ab43b..893b7e51cf25 100644 --- a/packages/utils/src/internals/sitemap.ts +++ b/packages/utils/src/internals/sitemap.ts @@ -385,47 +385,54 @@ export async function* parseSitemap( // the loop (per-entry warnings could flood the log; individual drops are logged at debug level). let droppedUrlEntries = 0; - for await (const item of items) { - if (item.type === 'sitemapUrl' && !visitedSitemapUrls.has(item.url)) { - if (nestedSitemapFilter && !nestedSitemapFilter(item.url)) { - logger?.debug(`Skipping sitemap ${item.url} due to nestedSitemapFilter.`); - continue; - } - - // Keep only nested sitemaps matching the strategy (and using http(s)) relative to the - // parent. Raw string sources have no parent URL, so the check is skipped. - if (source.type === 'url') { - const { allowed, reason } = filterUrl(item.url, sitemapUrl!, enqueueStrategy); - if (!allowed) { - logger?.warning(`Skipping nested sitemap ${item.url} (parent ${source.url}): ${reason}.`); + try { + for await (const item of items) { + if (item.type === 'sitemapUrl' && !visitedSitemapUrls.has(item.url)) { + if (nestedSitemapFilter && !nestedSitemapFilter(item.url)) { + logger?.debug(`Skipping sitemap ${item.url} due to nestedSitemapFilter.`); continue; } - } - sources.push({ type: 'url', url: item.url, depth: (source.depth ?? 0) + 1 }); - if (emitNestedSitemaps) { - yield { loc: item.url, originSitemapUrl: null } as any; - } - } + // Keep only nested sitemaps matching the strategy (and using http(s)) relative to the + // parent. Raw string sources have no parent URL, so the check is skipped. + if (source.type === 'url') { + const { allowed, reason } = filterUrl(item.url, sitemapUrl!, enqueueStrategy); + if (!allowed) { + logger?.warning(`Skipping nested sitemap ${item.url} (parent ${source.url}): ${reason}.`); + continue; + } + } - if (item.type === 'url') { - // Keep only URL entries that match the enqueue strategy relative to the parent (see above). - if (source.type === 'url') { - const { allowed, reason } = filterUrl(item.loc, sitemapUrl!, enqueueStrategy); - if (!allowed) { - droppedUrlEntries++; - logger?.debug(`Skipping sitemap URL ${item.loc} (parent ${source.url}): ${reason}.`); - continue; + sources.push({ type: 'url', url: item.url, depth: (source.depth ?? 0) + 1 }); + if (emitNestedSitemaps) { + yield { loc: item.url, originSitemapUrl: null } as any; } } - yield { - ...item, - originSitemapUrl: - source.type === 'url' - ? source.url - : `raw://${createHash('sha256').update(source.content).digest('base64')}`, - }; + if (item.type === 'url') { + // Keep only URL entries that match the enqueue strategy relative to the parent (see above). + if (source.type === 'url') { + const { allowed, reason } = filterUrl(item.loc, sitemapUrl!, enqueueStrategy); + if (!allowed) { + droppedUrlEntries++; + logger?.debug(`Skipping sitemap URL ${item.loc} (parent ${source.url}): ${reason}.`); + continue; + } + } + + yield { + ...item, + originSitemapUrl: + source.type === 'url' + ? source.url + : `raw://${createHash('sha256').update(source.content).digest('base64')}`, + }; + } + } + } catch (e) { + // Skip a malformed sitemap without losing the other ones; raw sources already report the error above. + if (source.type === 'url') { + logger?.warning(`Malformed sitemap content: ${source.url} (${e})`); } } diff --git a/test/utils/sitemap.test.ts b/test/utils/sitemap.test.ts index 72b5a7501760..68121ff63030 100644 --- a/test/utils/sitemap.test.ts +++ b/test/utils/sitemap.test.ts @@ -143,6 +143,21 @@ describe('Sitemap', () => { '', ].join('\n'), ) + .get('/sitemap_parent_with_malformed_child.xml') + .reply( + 200, + [ + '', + '', + '', + 'http://not-exists.com/not_actual_xml.xml', + '', + '', + 'http://not-exists.com/sitemap_child.xml', + '', + '', + ].join('\n'), + ) .get('/not_actual_xml.xml') .reply( 200, @@ -393,6 +408,22 @@ describe('Sitemap', () => { expect(sitemap.urls).toEqual([]); }); + it('keeps urls from other sitemaps when one of them is malformed', async () => { + const sitemap = await Sitemap.load( + ['http://not-exists.com/not_actual_xml.xml', 'http://not-exists.com/sitemap_child.xml'], + undefined, + { httpClient: new FetchHttpClient() }, + ); + expect(sitemap.urls).toHaveLength(5); + }); + + it('keeps urls from other nested sitemaps when one of them is malformed', async () => { + const sitemap = await Sitemap.load('http://not-exists.com/sitemap_parent_with_malformed_child.xml', undefined, { + httpClient: new FetchHttpClient(), + }); + expect(sitemap.urls).toHaveLength(5); + }); + it('handles CDATA in loc tags', async () => { const sitemap = await Sitemap.load('http://not-exists.com/sitemap_cdata.xml', undefined, { httpClient: new FetchHttpClient(),