A tester pinged me about a paging bug. Page two of a product list showed the same items as page one. Not similar, identical, right down to the order. I opened the endpoint, read the handler twice, and couldn't find anything wrong with it. It took a page number, skipped and took the right rows, returned them. On my machine it worked. In the deployed build it didn't.
The only thing I'd touched that week was adding output caching. That turned out to be the whole story.
What I thought I was doing
ASP.NET Core's output cache keys a response so a repeat request skips the handler. When I first wired it up it was just this:
app.MapGet("/products", (int page) => GetProducts(page)).CacheOutput();
And it behaved exactly how I wanted. Page one and page two cached separately. I know because I checked, more on that in a second. The default policy varies the cache key by every query string parameter. In the framework source it's literally context.CacheVaryByRules.QueryKeys = "*". So ?page=1 and ?page=2 land on different cache entries, which is the obvious right thing.
Then I wanted a 30-second expiry instead of the default, so I moved to a named policy. The endpoint also took an optional sort, so I added a SetVaryByQuery for it while I was in there. Felt thorough.
builder.Services.AddOutputCache(options =>
{
options.AddPolicy("short", b => b
.Expire(TimeSpan.FromSeconds(30))
.SetVaryByQuery("sort"));
});
That one line was the bug.
SetVaryByQuery replaces, it doesn't add
Here's the part that got me. I read SetVaryByQuery("sort") as "also vary by sort." It doesn't mean that. It means "vary by sort, and nothing else." It replaces the default "*" with the exact list you give it. The moment I named sort, I switched off varying by page, the one parameter that actually changed the response. And since no request even had a sort value, every call collapsed onto a single cache entry. First request in wins; everybody after gets its body handed back.
I didn't want to argue with myself about it, so I wrote the smallest thing that would prove it: a self-hosted minimal API that calls itself and counts how many times each handler really runs. If the run number repeats, the response came from cache. Nothing fancy, 127.0.0.1 and a loop.
builder.Services.AddOutputCache(options =>
{
options.AddPolicy("expire", b => b.Expire(TimeSpan.FromSeconds(30)));
options.AddPolicy("wrong", b => b.Expire(TimeSpan.FromSeconds(30)).SetVaryByQuery("sort"));
options.AddPolicy("page", b => b.Expire(TimeSpan.FromSeconds(30)).SetVaryByQuery("page"));
});
var hits = new ConcurrentDictionary<string, int>();
string Handle(string tag, int page) =>
$"page={page} handlerRun#{hits.AddOrUpdate(tag, 1, (_, v) => v + 1)}";
app.MapGet("/expire", (int page) => Handle("expire", page)).CacheOutput("expire");
app.MapGet("/wrong", (int page) => Handle("wrong", page)).CacheOutput("wrong");
app.MapGet("/page", (int page) => Handle("page", page)).CacheOutput("page");
Then I hit each one with ?page=1, ?page=2, and the same two again:
--- /expire --- (just Expire, no SetVaryByQuery)
asked page=1 -> "page=1 handlerRun#1"
asked page=2 -> "page=2 handlerRun#2"
asked page=1 -> "page=1 handlerRun#1"
asked page=2 -> "page=2 handlerRun#2"
--- /wrong --- (SetVaryByQuery("sort"))
asked page=1 -> "page=1 handlerRun#1"
asked page=2 -> "page=1 handlerRun#1" <-- WRONG PAGE
asked page=1 -> "page=1 handlerRun#1"
asked page=2 -> "page=1 handlerRun#1" <-- WRONG PAGE
--- /page --- (SetVaryByQuery("page"))
asked page=1 -> "page=1 handlerRun#1"
asked page=2 -> "page=2 handlerRun#2"
Look at the middle block. I asked for page two and got page one's body, handler-run number and all. That's the tester's bug, reproduced in about fifteen lines.
The real surprise for me was the top block. I'd assumed a named policy started empty and I'd have to opt back into varying by query. Nope. AddPolicy builds on top of the default policy, so /expire kept the "*" and paged correctly without me asking. The vary-by-query only broke once I named a key. The feature was protecting me right up until I decided to help.
The fix, and the knob I'd skip
The fix is boring: name every query key the response actually reads.
options.AddPolicy("short", b => b
.Expire(TimeSpan.FromSeconds(30))
.SetVaryByQuery("page", "sort"));
My rule now is to treat the arguments to SetVaryByQuery as the complete set of query parameters this endpoint depends on, not as "one more thing to vary by." Add a filter next month and it goes in that list too, or you ship this same bug again.
One thing I'd steer you away from. While I was digging I found AddPolicy(..., excludeDefaultPolicy: true). It sounds like a tidy clean-slate option, and I reached for it thinking it'd give me predictable behavior. What it actually did in my test was turn caching off completely, every request ran the handler and nothing got stored. That's a fine thing to want sometimes, but it's "opt out," not "start fresh," and the two are easy to mix up when you're poking at knobs late in the day.
Output caching earns its keep when a response is the same for everyone for a little while. The whole value lives in the cache key, and the cache key is made of the things you vary by. Get the vary-by wrong and you haven't made a slow endpoint fast. You've made a correct endpoint lie.
Have you read SetVaryByQuery the additive way too, or was that just me being tired? Curious how common that reading is.
— still poking at cache keys nobody asked me to poke at
Top comments (0)