Repository navigation
Seo/sitemap filter orphan entries - #12
Merged
Merged
Conversation
added 9 commits
August 31, 2026 14:45
The sitemap is proxied from VTEX, so it lists whatever the platform's category tree holds. Categories are created upstream by a different team than the one that builds the storefront pages, so an entry can point at a path no page serves — which answers 404. Announcing such a URL to crawlers is worse than omitting it. Behind `removeEntriesWithoutPage`, every <url> is now matched against the paths of the pages that actually exist, read with the same blockSelector website/loaders/pages.ts uses. Dropped entries are logged so the missing page can be created; the entry returns on its own once it is. The catch-all page is deliberately left out of the matchers: it matches every path, and counting it would make the filter inert. Also matches the storefront host instead of the exact configured URL when rewriting <loc>, so a variation the platform emits — http, or no trailing slash — cannot leak the platform host into the index.
Three defects from review, none of which the earlier commit could have shown working: The flag never reached the handler. The proxy loader builds the sitemap routes with a fixed set of props, and removeEntriesWithoutPage was not among them, so the handler always saw undefined and the filter never ran. It is now threaded through the loader to both sitemap routes. The matcher only imitated the router. Hand-rolled as a regex, it diverged on case, on trailing slashes and on percent-encoding: a page at /maçã does answer /ma%C3%A7%C3%A3 but would have been dropped, while /foo/ and /FOO, which answer nothing, would have been kept. It now builds the very same URLPattern the router builds, which also fixes a false positive found against production data — a page whose path carries a query string was being dropped, because the regex escaped the template whole. An empty page list disabled the filter, conflating a valid read with a failed one, which the catch already covers. The filter now always runs; what is refused instead is the narrower and unrecoverable case of dropping every entry, since an empty sitemap withdraws the whole site from the index and nothing downstream would notice.
Three defects from review. The rewritten body was returned under the upstream's framing headers. It is no longer the gzip stream content-encoding claims, nor the length content-length states — a mismatch that predates the filter, since rewriting the host already shortens every <loc>. Both headers are dropped from a clone of the response's. The guard against emptying a sitemap watched the symptom instead of the cause. Each child sitemap is handled on its own, so one made entirely of orphan entries is a legitimate emptying, and refusing it left those URLs published. What is unrecoverable is having no page to match against at all — that would empty every sitemap — so the guard now sits there. A <loc> holds XML and arrives entity-encoded, so a URL written as "?a=1&b=2" was compared literally against a route the router sees as "?a=1&b=2", dropping pages whose URL carries more than one parameter. Entities are decoded, numeric references included, before matching.
The sitemap is no longer rewritten. Removing entries meant reserializing a body the upstream's headers no longer described, and deciding on the storefront's behalf which URLs a crawler may see; a warning gives whoever can create the missing page the same information without either. The response goes back out exactly as it came in, and the prop says so. The check itself is now taken from the routes actually being served, read from the request state the way website/handlers/sitemap.ts does, rather than from the raw page blocks: a page hidden by hidePagesInDeco answers only with ?rdc=true, and reading its block would have called it reachable. Catch-all routes are recognized by probing what they answer instead of by their spelling, since the router normalizes "/*", "/(.*)" and an absolute "https://store.test/*" to the same pattern and any of them would have made the check vacuous. Routes with no pattern syntax — nearly all of them — are indexed in a set, so an orphan entry no longer walks every route: 10k orphan URLs against 500 routes went from seconds to 92ms, and the protocol allows 50k URLs per file. The log carries a count and a bounded sample instead of every URL, which on a large sitemap was megabytes to the console and the logger both. Finally, the host rewrite now requires the host to end where the match does, with an optional port. Unanchored, "shop.example.com" rewrote through the prefix of "shop.example.com.br" and mangled ":443" URLs.
Restores the removal. A URL the platform lists but no page serves answers 404, and announcing it to crawlers is worse than omitting it; it returns on its own once the page exists, since nothing here is persisted. Two defects from review, both of which removal turns from noise into damage. A catch-all was recognized by probing an improbable path, which "/:department/:category" also answers — every two-segment route would have been dropped from the index and every two-segment page erased from the sitemap. The router normalizes each spelling of a catch-all to the pathname "/*", so that is what is compared now. And the scan read every <loc> in the document, including the <sitemap> entries of an index, which name documents rather than pages: an external sitemap added through `include` would have been reported, and now removed. Matching whole <url> blocks confines the check to the urlset, which the removal needed anyway.
The response is decoded and rewritten before being served: the host is swapped in every <loc>, entries are dropped, includes are added. What the upstream sent about its own bytes stops holding — content-length is the length of a different document, content-encoding announces a gzip stream that was already decoded, and etag names the platform's version rather than the one being served, so a client could revalidate its way into a stale body. This predates the entry removal: rewriting the host alone already shortens every <loc>. Deleting the three from a clone lets the server frame the response it is actually sending. VTEX currently sends no etag for the sitemap, so that one is a precaution.
"/:path(.*)" and "/:path*" are catch-alls that normalize to themselves, not to "/*", so comparing the pathname let them into the index — and a route matching every URL makes the whole check vacuous. Probing a single path does not work either: "/:department/:category" answers two segments of anything and would be mistaken for one. What separates a catch-all from every other route is that it answers paths of differing depth, so that is what is asked of it. This recognizes "/*", "*", "/(.*)", "/:path(.*)", "/:path*" and the absolute form, while "/loja/:path*" — a catch-all under a prefix, which does not match every URL — stays an ordinary route.
Forwards only the headers still true of a document this handler rewrote — content-type, cache-control and vary — instead of deleting the ones known to be wrong one at a time. The upstream's content-length counts bytes of another document, content-encoding announces a gzip stream reading the body already undid, etag and last-modified name the platform's version rather than the served one, and accept-ranges offers ranges over offsets into a document nobody will receive. The list also stops the next such header from having to be found in production first. This drops the platform's x-vtex-* cache diagnostics from the response, which described the upstream fetch rather than what is served. A 304 is also returned untouched now. It carries no body to rewrite, and handing one to a Response throws outright: "Response with null body status cannot have body". 204 and 205 are covered by the same rule.
The previous guard named the statuses that carry no body, which left the ones carrying the wrong body. A 206 was rewritten as though its fragment were a whole sitemap, and a 301 or 302 came out without its Location, since a forwarded header list cannot carry what it does not name — a redirect the earlier code, copying every header, had passed through intact. An error body was being parsed as XML too. Only a complete document can be rewritten as one, so only a 200 is.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What is this Contribution About?
Please provide a brief description of the changes or enhancements you are proposing in this pull request.
Issue Link
Please link to the relevant issue that this pull request addresses:
Loom Video
Demonstration Link