DEV Community

Daniel Pertu
Daniel Pertu

Posted on

Four things can close the same quiz question, and three of them have to lose

In a live pub quiz, a question closing is the most contested event in the system. Four separate things in our stack are allowed to ask for it:

  1. The timer on the WebSocket server, armed when the question was launched.
  2. The host, who has a button and does not always wait for the timer.
  3. The early close, which fires when every phone in the room has answered.
  4. A self-healing sweep on the WebSocket server, which notices a question whose time ran out while nobody was listening.

On a good night, exactly one of those happens. On a real night, two of them happen inside the same hundred milliseconds, because the last team answered just as the clock hit zero and the host pressed Next because he could see they had.

The naive version of this code closes the question four times.

What four closes actually did

Our close path does more than stamp a timestamp. It marks the question closed, re-ranks the leaderboard, broadcasts the result to every phone, and decides whether the session is over. Running it twice meant all of that twice:

  • closed_at was re-stamped, so the second close overwrote the first with a slightly later time.
  • The results broadcast went out twice, and every phone re-animated the same reveal.
  • In auto-advance mode, a second auto-advance was scheduled. Two timers now wanted to launch the next question.

And launch had the matching problem in the other direction. The host pressing Next while the auto-advance fired re-launched the question that was already on screen, which reset its clock under the players' feet. The room sees fifteen seconds jump back to fifteen seconds and a table that had already answered is told to answer again.

None of this is a crash. Nothing shows up in error monitoring. It is just a quiz night that behaves strangely in front of fifty people, which is worse.

The fix is not a lock, it is a from-state

The instinct is to reach for a mutex. We did take the row lock, and it is necessary, but it is not the interesting part. A lock only serialises the four callers. Serialised, they still all run.

What makes the three losers harmless is that every transition states which state it is allowed to move from, and checks it inside the lock.

Launch has exactly two legal moves:

if (session.status === 'setup') {
    if (questionIndex !== 0) return GameError.INVALID_TRANSITION
} else if (session.status === 'active') {
    const current = findQuestionState(states, session.current_question_index)
    if (!current?.closed_at || questionIndex !== session.current_question_index + 1) {
        return GameError.INVALID_TRANSITION
    }
} else {
    return GameError.INVALID_TRANSITION
}
Enter fullscreen mode Exit fullscreen mode

Read that as a sentence: a session in setup may open question zero, and an active session may open the question after the current one, but only once the current one has closed. There is no third option. A duplicate launch of the question already on screen fails the second branch, because that question's state has no closed_at. It returns an error code and does nothing. The host's extra click is not an error the room can see, it is a no-op.

Close is the same shape:

if (session.status !== 'active') return GameError.INVALID_TRANSITION
const state = findQuestionState(states, currentIndex)
if (!state?.launched_at || state.closed_at) return GameError.INVALID_TRANSITION
Enter fullscreen mode Exit fullscreen mode

A question that is already closed has a closed_at, so the second, third and fourth close all stop here, before the re-rank, before the broadcast, before the auto-advance gets scheduled. The whole doubled-reveal class of bug is that one condition.

All of it sits inside a transaction that selected the session row for('update'), which is what stops two callers both reading "not closed yet" and both proceeding. The lock makes the check trustworthy. The check is what makes losing harmless.

The guard for timers that fire late

There is one more race that a from-state check does not catch, and it is the nastiest, because it produces an outcome that is individually legal at every step.

A timer armed for question 3 fires late. By the time it lands, question 3 has already been closed by the host and question 4 has been launched. The session is active, question 4 is open and has no closed_at. Every condition above passes. The late timer closes question 4, about one second into its fifteen.

So a timer has to say which question it was armed for:

const currentIndex = session.current_question_index
if (opts.expectedIndex !== undefined && opts.expectedIndex !== currentIndex) {
    return GameError.INVALID_TRANSITION
}
Enter fullscreen mode Exit fullscreen mode

expectedIndex is optional on purpose. The host's button genuinely means "close whatever is open", because the host is looking at the room. A timer never means that. It means "close the question I was counting down for, if it is still the one on screen". Our question timer only ever ends the countdown it started, and that line is the whole reason.

Ending the session is one UPDATE

The end transition is the only one that does not need a read at all, and it turned out to be the cleanest of the three. Instead of select, check, then write, the check goes into the WHERE:

const [ended] = await tx
    .update(gameSessionsTable)
    .set({ status: 'completed', ended_at: new Date() })
    .where(
        and(
            eq(gameSessionsTable.id, sessionId),
            inArray(gameSessionsTable.status, ['setup', 'active']),
        ),
    )
    .returning({ id: gameSessionsTable.id })
Enter fullscreen mode Exit fullscreen mode

Postgres will only match a row that is still in setup or active, so two concurrent ends cannot both get a row back. The one that matched nothing then does a second read purely to produce a decent error code: if the session exists, the caller raced and gets invalid_transition; if it does not, they get session_not_found. That distinction costs one extra query on the failure path only, and it is the difference between a host seeing "already finished" and seeing "something went wrong".

Every one of these returns, none of them throw

All three transitions hand back an ActionResult<T> and never throw at the UI:

export const GameError = {
    INVALID_INPUT: 'invalid_input',
    INVALID_TRANSITION: 'invalid_transition',
    SESSION_NOT_FOUND: 'session_not_found',
    RATE_LIMITED: 'rate_limited',
    // ...
} as const
Enter fullscreen mode Exit fullscreen mode

String-literal codes rather than exceptions, because the four callers want different things from a failure. The host's UI wants to show a message. The WebSocket timer wants to shrug and move on, since invalid_transition is its normal answer when the host got there first. A thrown error makes those the same event, and the one the timer gets most often would have been the loudest.

The rate limiter sits in front of all three, keyed on the session rather than the user:

if (!(await ratelimit.hostAction.limit(`host:${sessionId}`)).success) {
    return { success: false, error: GameError.RATE_LIMITED }
}
Enter fullscreen mode Exit fullscreen mode

Keyed on the user, the WebSocket server's auto-advance would share a bucket with every other session it is running. Keyed on the session, the limit is well above any real quiz and exists only to stop a loop, which is a thing two components that can each trigger the other are genuinely capable of.

The authorisation bug underneath all of it

Worth saying plainly, because it is the actual lesson. These functions used to be exported Server Actions, called straight from the host's React components, with no caller check inside them.

A Server Action is an HTTP endpoint. Every player in the room holds the session id, because the session id is how they joined. Anyone who looked at a network tab could have closed questions or ended the quiz.

They are now plain server functions. They are not reachable from a browser at all. The host-facing path goes through an action that first checks the caller actually hosts that session, and the WebSocket server's timer reaches them through internal routes behind a shared secret. The transition guards are correctness; moving the functions out of the Server Action namespace is the security fix, and the two are easy to confuse because both are about "who is allowed to close this question".

What this looks like from the floor

None of this is visible when it works, which is the point. The host presses Next twice and the quiz advances once. The last team answers as the clock expires and the room gets one reveal, not two. A deploy lands mid-question and the sweep that picks it up afterwards does not re-close something the host already handled.

If you want to see the surface these guards sit under, the quiz builder is where a host sets each question's own answer window, five to fifteen seconds, which is the number the timer is armed with and therefore the number a late timer would wrongly cut short. And the live leaderboard is the thing that recomputes on every close, which is why closing twice was never a cosmetic problem.

The general shape, for any system where several components can request the same transition: do not try to make sure only one of them asks. You cannot. Make the question "what state am I moving from" instead of "what state am I moving to", check it under the lock, and let everybody ask.

Top comments (0)