Skip to main content

Command Palette

Search for a command to run...

The CORS Error That Was a Race Condition

The browser blamed CORS. The CORS config was fine. The real fault was two tabs refreshing the same token at the same moment.

Updated
16 min readView as Markdown
The CORS Error That Was a Race Condition

The browser console said the request had been blocked by CORS policy, that no Access-Control-Allow-Origin header was present on the requested resource. Which is a sentence I have read several hundred times and always means the same thing.

Except it didn't, this time. The CORS configuration was correct. It had been correct all this time. Every other endpoint on the same origin, through the same middleware, with the same credentials, worked. Only /auth/refresh failed, and only sometimes.

I went to the CORS config first anyway. Of course I did. That's what the error said.

TL;DR

An unhandled exception produced a 500 that never passed back through the middleware which attaches CORS headers, so the browser saw a header-less response and reported the only thing it could: a CORS failure. The message named the last layer to touch the response, not the layer that broke.

Underneath was a textbook time-of-check-to-time-of-use race in refresh-token rotation. Two tabs, one token, both passing the same "is this blacklisted?" check before either had written its answer.

The fix wasn't a better check. It was giving up on checking, letting a unique index arbitrate, and translating the resulting error. Three separate races in this codebase are now settled the same way.

This codebase was deliberately vibe coded, as a test of how far that approach goes without close architectural steering. This was a different kind of result from the cost one. That constraint lived outside the repository and no amount of reading code would have surfaced it. This one is four lines of source that anyone can read, and it got through anyway.

Why a 500 arrives as a CORS error

Worth understanding on its own, because it will cost you an afternoon exactly once and then never again.

CORS headers are attached by middleware. Middleware sits in a stack, each layer wrapping the next, and a response on the way out passes back through every layer that it passed through on the way in. That's how the header gets on there.

An unhandled exception does not come back out that way. It propagates up to the outermost error handler, which generates a 500 above the CORS layer, so the response the browser receives never went through the code that would have added Access-Control-Allow-Origin.

From the browser's side, a response with no CORS header is indistinguishable from a response that was refused on CORS grounds. It has no way to know the server crashed. It reports what it can see, which is a missing header, and you go and read documentation about preflight requests for forty minutes.

The general form is worth keeping:

When a browser reports a CORS failure on an endpoint whose CORS config you have not touched, read the server log before you read the CORS docs. The browser is describing the shape of the response, not the cause of it.

Worth separating two problems here, because fixing the race only fixes this particular 500. The general fix is a global exception handler that catches anything unhandled and returns a real response from inside your own middleware, so the response travels back out through the whole stack and arrives at the client with headers on it. The framework's own error handler sits above everything you install, which is why responses it generates look nothing like your others. You want the frontend reading a 500 and failing gracefully, not dying on what it thinks is a network error.

A request passing inward through CORS, compression, logging and idempotency middleware to a route that throws, with the resulting 500 generated above the CORS layer and reaching the browser without an Access-Control-Allow-Origin header

The server log said IntegrityError. Nothing to do with CORS at all.

What the refresh endpoint was doing

The auth design is short-lived access tokens with rotating refresh tokens. Each refresh consumes the token it was given and issues a new one, and the consumed token goes into a blacklist table keyed by its jti, its unique token identifier. A refresh token is therefore valid exactly once. Present it twice and the second attempt is refused.

That refusal is deliberate, and it isn't only bookkeeping. A rotating refresh token that gets replayed is one of the few signals you get that a token has been stolen, because the legitimate client and the attacker will both eventually present the same one. Rejecting reuse is the entire point of rotation.

The endpoint did the obvious thing:

if await blacklist.contains(jti):
    raise Unauthorized("token already used")

new_tokens = issue_pair(user)
await blacklist.add(jti)
return new_tokens

Read it and it looks correct. It reads correct out loud. Check whether the token has been used, and if it hasn't, use it.

Two tabs

The user has the application open in two tabs. Both hold the same refresh token, because it's persisted so a reload can recover the session. The access token expires. Both tabs notice at approximately the same moment and both call /auth/refresh.

Or it isn't two tabs at all, it's one tab and a retry, or a client that fired the refresh twice while the first request was still in flight. It doesn't matter which. What matters is two requests carrying the same jti, close enough together to overlap.

Two overlapping requests: both query the blacklist and find nothing, both proceed, the first insert succeeds and the second collides with the unique index on jti

Request A checks the blacklist. Not there. Request A proceeds.

Request B checks the blacklist. Also not there, because A hasn't written its row yet. Request B proceeds.

Both issue a new token pair. Both then try to insert the same jti. The first insert lands. The second hits the unique index and Postgres refuses it, SQLAlchemy raises IntegrityError, nothing catches it, and it becomes a 500 that arrives in the browser wearing a CORS costume.

This is time-of-check-to-time-of-use, and it's one of those bugs that's completely obvious in retrospect and completely invisible in review, because the code says what you meant. The gap between the check and the write is where the whole thing lives. On a local machine with one tab, that gap is microseconds and you will never see it. Under any real concurrency, it's just a window, and windows get walked through.

You cannot fix this with a better check

The instinct is to tighten the check. Move it closer to the write, wrap it in a transaction, add a lock. Some of that helps. None of it addresses the shape of the problem.

SELECT then INSERT is two statements. Whatever you do between them, there is a moment when the first has returned and the second has not yet run, and in that moment another connection can be doing precisely the same thing. You can make the window narrower. You cannot make it not exist, because it is made of the fact that you asked a question and then acted on the answer.

You can go get a lock, and for some problems that's right. But you've now introduced a lock into your hottest auth path to defend against something the database was already going to catch for you, because the unique index on jti was there the whole time. It was doing its job perfectly. The only thing wrong was that nobody was listening to it.

Let the database decide

The fix is to stop asking and start attempting:

new_tokens = issue_pair(user)
try:
    await blacklist.add(jti)
except IntegrityError as e:
    await session.rollback()          # the transaction is already aborted
    if "ix_blacklist_jti" in str(e.orig):
        raise Unauthorized("please log in again")
    raise                             # a different constraint is a different bug
return new_tokens

The unique constraint is the arbiter. Exactly one insert can win, the database guarantees it under any amount of concurrency, and the loser gets told so. All that's left is translating a database error into an HTTP response.

The part I find satisfying is that the translation was already decided for us. A duplicate jti means this refresh token has been presented twice. The correct response to a replayed refresh token is to refuse it and make the user authenticate again. So the answer to the race and the answer to a stolen token are the same answer, and the endpoint doesn't need to know which situation it's in. It just needs to be honest that this token is no longer good.

One detail in there is deliberate and looks wasteful. The new token pair is issued before the insert is attempted, so the losing request does work it then throws away. That's fine, and the alternative is worse. Blacklisting first would mean consuming the old token before knowing whether you can successfully issue a replacement, and a failure between those two points logs the user out for no reason at all. Doing the cheap, reversible work first and letting the irreversible step be the one that can fail is the ordering you want whenever you can get it.

The 500 became a 401. The CORS error disappeared, because there was no longer an unhandled exception, and the browser started receiving a response that had passed through the whole middleware stack on its way out.

Three races, one pattern

Once the shape is visible you start seeing it. There were three of these in the codebase, and they're now all settled the same way:

Three races and their arbiters: a unique jti on the token blacklist yielding a 401, a composite unique on user and story yielding a 409, and an optimistic lock column yielding a 409

Concurrent refresh rotation is arbitrated by UNIQUE (jti) on the blacklist table. IntegrityError becomes 401, log in again.

Double like or bookmark is arbitrated by a composite UNIQUE (user_id, story_id). This is the same bug in a friendlier costume. Someone double-taps a heart on a slow connection, or the client retries a request whose response got lost, and two inserts arrive for a row that should exist at most once. The application version of this guard reads "if the user has already liked it, return early," and it passes for both racers for exactly the same reason the token check did. The composite unique means the database enforces one like per user per story and the application stops pretending to. IntegrityError becomes 409, and the client, which was going to show a filled-in heart either way, carries on.

Concurrent story edits are arbitrated by an optimistic lock. The stories table carries a row_version column wired to SQLAlchemy's version_id_col, so every update includes the version it read and increments it. Two editors open the same story, both save, and the second UPDATE matches zero rows because the version moved underneath it. SQLAlchemy raises StaleDataError, which becomes a 409 with "this was edited while you were working."

This third one is the one I'd argue hardest for, because its failure mode is the quietest. Without the version column, both saves succeed. The second UPDATE writes the whole row over the top of the first, both requests return 200, both editors see a saved state, and the earlier person's paragraph is simply gone. Nothing errors. Nothing logs. There is no artifact anywhere in the system indicating that an edit was destroyed, and the only way anyone finds out is a human noticing their own work missing and not being believed. A 409 is a small inconvenience by comparison, and it's an inconvenience that arrives at the moment it can still be resolved.

Same structure three times. Don't prevent the race, define what winning means as a constraint, let the database enforce it, and translate the failure into something the client can act on.

The same experiment, a different kind of failure

I should say where this code came from, because it changes what the bug means.

I deliberately vibe coded most of this project. I wanted to test the claim that coding agents can build a full-stack app almost end to end, and more importantly to find where that approach failed when nobody was carefully steering the architecture. It's a personal project, which made it a safe place to let the experiment break things.

The first result I got from that was an operational one: an architecture that was entirely conventional and entirely correct, carrying a running cost that appeared nowhere in the source tree.

This one is different, because the defect is in the repository. It's four lines. And it still gets through everything. It type-checks. It can pass review, because there's nothing sloppy to point at. It passes the test suite, and it would pass a test suite twice the size, because the bug does not exist unless two requests overlap and nothing about a normal test run produces overlap. Sequential tests pass. Manual testing passes. Local development, one tab, one request at a time, passes forever.

Check-then-act is the shape you reach for by default, and that's the actual finding. It's how the requirement sounds when you say it out loud. "Don't allow a token to be used twice" becomes "check if it's been used, and if it has, refuse," and the translation from English into code feels lossless. Plenty of straightforward examples use that shape for the same reason, because it mirrors the requirement. It's the readable version, the version that explains itself, and it encodes a single-threaded picture of the world that stops being true the moment two people are using your application.

An agent reproduces convention faithfully, which is usually what you want and is exactly the problem here, because the convention is wrong. There's no sloppiness to catch. It's a correct-looking implementation of a subtly wrong idea about how time works.

A strong reviewer can catch this, and some would have. But ordinary review reads for what the code says, and catching this one requires reading for what two copies of it would do at the same moment, which is a different and more deliberate act. A concurrency test would have made it deterministic instead of leaving it to whether the reviewer happened to be in that mode that day.

What this costs you

Not free, and it's worth naming the price rather than presenting this as a pure win.

Your error translation layer becomes load-bearing. Before, an IntegrityError was an accident. Now it's part of the control flow, which means an uncaught one is a 500 you have chosen to have. Every constraint you rely on needs a handler, and the handler needs to be on the path the error actually takes.

You have to manage the transaction state. When Postgres raises an IntegrityError it aborts the current transaction. Catch it, return a response, and that connection goes back to the pool still in an aborted state, at which point the next request to pick it up dies immediately with PendingRollbackError. The failure lands on a completely unrelated request, which makes it thoroughly unpleasant to track down. Roll back explicitly in the handler.

You have to know which constraint fired. Catching IntegrityError broadly and returning 401 is fine when the table has one unique index. When a table has several, a bare except IntegrityError will happily translate a completely unrelated violation into "please log in again," which is a worse bug than the one you fixed because it's misleading. Match on the constraint name.

The error path needs the same care as the happy path. Which it always did. The difference is that now it's obvious.

And it reads as less safe than it is. A try/except around a write looks like error handling bolted on. The check looked like a real guard. The intuition is backwards, and if you're working in a team, this is a code-review conversation you'll have more than once.

What I got wrong

I spent the first stretch of this in the wrong layer entirely, reading about preflight requests and credentialed origins, because the error message told me to and I believed it. The server log had the answer in it the whole time. I hadn't looked, because the browser had already told me what was wrong with such apparent confidence.

The second thing, and it's the one I'd actually change about how I work: I knew what TOCTOU was. I could have defined it. That didn't help even slightly, because knowing the name of a bug class and recognising it in code you wrote yesterday are unrelated skills. What made it visible was not knowledge, it was reading the traceback and asking why a unique constraint could possibly be violated on a table where the application had just verified the row was absent. That question only has one answer.

Start smaller than this

If this is new, you don't need to go and audit everything. One question is enough, and you can ask it about a single endpoint:

Between the moment this code checks a condition and the moment it acts on it, could another request have changed the answer?

If it could, find out what already guarantees correctness underneath. Usually something does, and usually it's a constraint that's been sitting in your schema since the first migration, quietly doing its job while the application asks it questions it already knows the answer to.

You don't have to restructure anything today. Knowing which of your guards are real and which are polite suggestions is most of the value.

What carries over

  1. A CORS error on an endpoint whose CORS config you haven't touched is probably not a CORS error. Read the server log first.
  2. Middleware attaches response headers on the way out, and an unhandled exception doesn't take that route. Every error handler you rely on lives at a specific place in the stack, and errors that escape it produce responses that look nothing like your others.
  3. SELECT then INSERT is two statements, and the gap between them is real. You can narrow it. You cannot close it.
  4. A unique constraint is a decision, not a safety net. If the database already guarantees the thing you're checking, the check is duplicated logic that's wrong more often than the constraint is.
  5. Attempt, catch, translate. Let the write fail and turn the failure into an HTTP status the client can act on.
  6. Match on constraint names when a table has more than one. A broad except IntegrityError will confidently return the wrong error.
  7. Check-then-act passes every test that doesn't have concurrency in it, which is most tests, which is why this survives review and reaches production.

The database was going to enforce it either way. The only question was whether the application found out politely, or as a 500 dressed up as somebody else's problem.


I'm Ashish Kr Jha, a software engineer. I build things in production and then have to live with them, which is the part that actually teaches you something. More at ashishkrjha.dev.