DEV Community

Daniel Pertu
Daniel Pertu

Posted on

null meant unlimited, so for a while our free trial could build more question packs than Pro

Here is a bug that passes review, passes types, and reads correctly in both of the places it is wrong.

trial: {
    // ...
    maxCustomPacks: null,  // no custom packs on the trial
},
pro: {
    // ...
    maxCustomPacks: 5,
},
Enter fullscreen mode Exit fullscreen mode

The interface says what null means, two lines above:

/** Max custom question packs. null = unlimited */
maxCustomPacks: number | null
Enter fullscreen mode Exit fullscreen mode

So the trial had unlimited custom question packs and Pro, which people pay £30 a month for, had five. The comment on the trial row said the opposite, which is why nobody caught it by reading.

We sell quiz software to pubs on three plans. This is a post about the shape of the limits table behind that, the sentinel that made this possible, and where a plan change turns into enforcement.

One table, six limits, three plans

export interface PlanLimits {
    /** Max simultaneously active quiz sessions for this subscription */
    maxConcurrentSessions: number
    /** Max quizzes that can be started per calendar day (UTC). null = unlimited */
    maxSessionsPerDay: number | null
    /** Max active tables in the venue. null = unlimited */
    maxTables: number | null
    /** Max custom question packs. null = unlimited */
    maxCustomPacks: number | null
    /** Max accounts (owner + invited hosts) per venue */
    maxAccounts: number
    /** Max concurrent logged-in browser sessions per individual user account */
    maxConcurrentLogins: number
}
Enter fullscreen mode Exit fullscreen mode

Six numbers, and the first thing to notice is that only three of them are nullable. maxAccounts and maxConcurrentLogins are plain numbers because unlimited is not a thing we would ever want to offer there: unlimited seats is a pricing mistake, and unlimited concurrent logins is a password being shared with a WhatsApp group. Making a field non-nullable is the cheapest way to say that out loud.

Seven call sites read this table: creating a pack, adding a table, starting a quiz, inviting a host, the settings page, the Stripe webhook, and the Redis session cap writer. None of them has its own copy of a number.

The check is skipped, not failed, when the value is null

Here is why the null bug was invisible:

if (limits.maxCustomPacks !== null) {
    const [{ packCount }] = await db
        .select({ packCount: count(questionPacksTable.id) })
        .from(questionPacksTable)
        .where(eq(questionPacksTable.created_by, user.id))

    if (Number(packCount) >= limits.maxCustomPacks) {
        throw new Error(/* ... */)
    }
}
Enter fullscreen mode Exit fullscreen mode

null does not mean "the limit is zero" or "compare against null and see what happens". It means the entire check, including the count query, does not run. Both call sites are written that way, and that is the correct implementation of the documented contract. Which is exactly the problem: the code was right, the contract was right, and the data was wrong, so there was nothing for a type checker or a reviewer to catch.

A sentinel value that means "skip the enforcement" is a loaded gun pointed at whichever row somebody fills in carelessly. Two things would have prevented this and the one we chose is the boring one.

The fix is 0:

// 0, not null. The contract above is "null = unlimited", and both call
// sites skip the check entirely when the value is null, so the comment
// said "no custom packs" while the value said "unlimited", and a trial
// account could create more packs than a paying Pro one (capped at 5).
maxCustomPacks: 0,
Enter fullscreen mode Exit fullscreen mode

The comment is long because it is the kind of thing somebody will "tidy up" back to null in a year, and a comment that only says what the value is gives them no reason not to.

The option we did not take was replacing the sentinel with a tagged union, something like { kind: 'unlimited' } | { kind: 'capped', max: number }. That makes the bug unrepresentable, and for six fields on three plans it is more ceremony than the problem is worth. The honest reason is the ratio: this table is read far more often than it is written, and a union makes every read site more verbose to protect the handful of lines that are ever edited. If it were twenty limits across eight plans I would feel differently.

Zero needs its own sentence

Once 0 is a real value, the error message has to deal with it:

throw new Error(
    limits.maxCustomPacks === 0
        ? 'Custom question packs are not included in the trial. Upgrade to Pro to build your own.'
        : `You've reached the ${limits.maxCustomPacks}-pack limit on your plan. Upgrade to Ultimate for unlimited packs.`,
)
Enter fullscreen mode Exit fullscreen mode

"You have reached the 0-pack limit on your plan" is what you get for free, and it is the sort of copy that makes a product feel unfinished. The branch is not about grammar: zero is a different situation from five. At zero the feature is not included and the fix is to upgrade to the cheapest paid plan. At five the feature is included and you have used it up. Those are two different messages because they are two different problems.

The daily quiz cap has the same shape and the same null skip:

if (limits.maxSessionsPerDay !== null) {
    const todayUtc = new Date()
    todayUtc.setUTCHours(0, 0, 0, 0)
    // count this venue's sessions since midnight UTC
}
Enter fullscreen mode Exit fullscreen mode

Note that the day is UTC, and the field comment says so. For a product sold to pubs in the UK that is correct for most of the year and wrong for the half of it that is on BST, where a quiz starting at midnight counts against the previous day. It is in the table as a known, documented coarseness rather than a thing we got away with: the cap is 5 a day on Pro, nobody runs five quizzes in a night, and the fix is a venue timezone column we have not needed yet.

The webhook is where a plan change becomes enforcement

A subscription changing in Stripe is the only event that moves a venue between rows of that table, so the webhook is where the consequences get applied:

const limits = getPlanLimits(planType)
await enforceNewSeatLimit(venue.id, limits.maxAccounts)
await setUserSessionCap(userId, planType)
Enter fullscreen mode Exit fullscreen mode

Two different kinds of limit, handled two different ways. Seats are in Postgres and have to be reconciled at the moment of the change, because a downgrade from five accounts to one has four people who can currently sign in and should not be able to. Pack and table limits need none of that: they are checked on creation, so an account that is over the new limit simply cannot add more, and nothing has to be taken away retroactively.

The session cap is in Redis, written by the webhook and read by the auth path on every dashboard request:

const CAP_TTL_S = 60 * 60 * 24 * 35

await redis.set(capKey, limits.maxConcurrentLogins, { ex: CAP_TTL_S })
Enter fullscreen mode Exit fullscreen mode

Thirty-five days, which is deliberately longer than any billing cycle. The TTL is not an expiry policy, it is garbage collection for users who stop existing: the webhook rewrites the key on every subscription change, so a live account's key is never allowed to lapse, and a dead account's key clears itself out rather than sitting in Redis forever. The reader falls back to a literal 3 when the key is absent, which is the value both paid plans carry, so a missed write degrades to the common case rather than locking anybody out. That makes the whole thing eventually consistent rather than something that breaks when a write does not land.

For the Ultimate plan the webhook also walks the venue's invited hosts and updates each of their caps, because the cap is per user account and the plan is per venue. That is a loop in a webhook handler, which I do not love, and it is bounded by maxAccounts: 5, which is the non-nullable field doing its second job.

The marketing copy is deliberately not derived

The obvious next step is to generate the feature comparison on the pricing page from this table. We did not, and I think that is right.

/**
 * Hosts and venues. The numbers are the Ultimate row of utils/plan-limits.ts,
 * maxAccounts 5 and maxConcurrentSessions 5, and the invite flow described is
 * app/dashboard/settings/team-actions.ts.
 */
Enter fullscreen mode Exit fullscreen mode

That is the top of a public feature page, and the numbers in its prose are typed out by hand. A derived page would say "5 accounts". The hand-written one says this:

Pro is one account: one person signs in and that person hosts. Ultimate is up to five, invited by email from your settings, each able to run a quiz in their own right.

This is the difference that usually decides the plan, and it is rarely about scale. It is about not sharing a password with the three people who cover Tuesdays.

You cannot generate the second paragraph from a number, and the second paragraph is the one that sells the plan. A table of limits renders as a table of limits, and a table of limits is what every competitor's pricing page already is.

What the comment buys is the thing derivation would have given us: when somebody changes maxAccounts, grep finds the page that has to change with it. That is a weaker guarantee than a compile error, and it is the right strength for copy, because the copy needs rewriting rather than re-rendering when the number moves.

The prices are a different matter and those are derived:

export const PLAN_PRICES_PENCE: Record<PlanType, number> = {
  trial: TRIAL_PRICE_PENCE,
  pro: PRO_PRICE_PENCE,
  ultimate: ULTIMATE_PRICE_PENCE,
}
Enter fullscreen mode Exit fullscreen mode

Keyed by the same PlanType as the limits table, so a new plan cannot be added to one and forgotten in the other, and held in pence rather than as a display string because a hardcoded "£30" in a component cannot be converted into another currency and cannot be checked against anything.

Have a look

pub-trivia.app/pricing is the customer-facing end of the table. The cards are a tick list, which is the one place on the site where the limits genuinely do read as a table, because that is what somebody comparing two plans is there to do. The number that was wrong in this post is the custom packs line: Pro reads "Up to 5 custom question packs", Ultimate reads "Unlimited custom question packs", and the trial section of the FAQ below says plainly that custom packs of your own are one of the two things it does not include.

That FAQ answer is the prose version of the same table, and it is worth comparing the two on one page: the ticks let you scan, and the paragraph is where the limits get explained rather than listed.

pub-trivia.app/features/multi-venue is the page whose source comment is quoted above, so you can read the prose that a generated page would have replaced and decide whether you agree with me.

The free tier needs no card, and it is the trial row of that table, now with zero custom packs rather than unlimited ones.

Top comments (0)