Answer 404 for section slugs that don't exist - #208
Merged
Conversation
A request for a section or subsection that isn't in site_taxonomy came back 400. The slug is a path parameter on these two routes, so an unknown value is a missing resource, not a malformed request — and Delta was logging around a thousand of these a day, which kept the error-rate panel near 5% and made it useless for alerting. The validator is shared with the query-string filters on GET /v1/articles, where 400 is still right, so it now returns sentinels and each handler decides the status for the slug it took from its own path. The message strings are unchanged; only the code moves. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
The production error-rate panel has been sitting near 5% and is useless as an alerting signal, because the largest single contributor is not an error. Over 6h on Delta today: 720
400 GET /v1/sections/{section_slug}/articlesand 326400 GET /v1/subsections/{subsection_slug}/articles, roughly a thousand a day.The slug is a path parameter on both routes, so a value that isn't in
site_taxonomyis a missing resource, not a malformed request. It now returns 404.Why it isn't a one-line status change
normalizeAndValidateArticleParamsis shared withGET /v1/articles, wheresection_slugandsubsection_slugare query filters the caller chose rather than the resource being addressed — 400 is correct there. So the validator returns two sentinels,errSectionNotFoundanderrSubsectionNotFound, and each handler passes the one whose slug came from its path:GET /v1/sections/sports/articles?subsection_slug=bogustherefore still answers 400. So doessubsection_slug does not belong to section_slug, which is a genuinely contradictory request.The error message strings are unchanged, so response bodies are byte-identical and only the status code moves. That matters for the public site:
getSectionArticlesdoesif (!res.ok) return;, treating 400 and 404 the same, so this ships independently of anything on Scalene.Where the volume actually comes from
Two client-side causes, both tracked on the Scalene board rather than fixed here:
/sections/first and falling back to/subsections/, so every subsection page burns one failing request by design. Confirmed forpolitics,music,gaming,cooking,world./sw.js404s in production, falls through to the same catch-all, and gets looked up as a section — 126 of these in 90 minutes. The service worker isn't registering at all, which is a real bug independent of the metrics.Three slugs 400 on both endpoints, meaning they're genuinely absent from
site_taxonomy:podcasts,sjn-grant,tri-this-sweet-treat. Those need separate triage; this change makes them 404 instead of masking them as bad requests.Deliberately out of scope
GET /v1/articles/{slug}returns 400 for values likeindex.xml. That slug really is syntactically invalid, andisValidCanonicalSlughas 25+ call sites that are mostly write endpoints where 400 is plainly right — not worth widening this diff for ~5 events per 6h.Testing
go test ./...andgo vet ./...clean. No existing test asserted 400 on these routes, so nothing needed loosening. Added coverage for both sentinels, the full status-mapping table, handler-level 404s, and the query-filter-stays-400 case. Swagger regenerated — additive404on these two routes only.The taxonomy lookup can't be exercised locally (DB1's grants are host-scoped), so the handler tests run against the no-database fallback, which reaches the same sentinel. Real confirmation is the error-rate panel after deploy.
🤖 Generated with Claude Code