DEV Community

Daniel Pertu
Daniel Pertu

Posted on

null meant unlimited, so our free trial outranked the paid plan

#webdev #programming #typescript

Here is a shape you have written. I have written it a dozen times.

export interface PlanLimits {
  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
  maxAccounts: number
  maxConcurrentLogins: number
}
Enter fullscreen mode Exit fullscreen mode

number | null, where null means "no limit". It reads fine. It is compact. Every call site does the obvious thing:

if (limits.maxCustomPacks !== null) {
  const packCount = await countPacksForVenue(venueId)
  if (Number(packCount) >= limits.maxCustomPacks) {
    return { error: 'Pack limit reached' }
  }
}
Enter fullscreen mode Exit fullscreen mode

And then the table itself:

trial: {
  maxConcurrentSessions: 1,
  maxSessionsPerDay: 1,
  maxTables: 10,
  maxCustomPacks: null, // no custom packs on the trial
  maxAccounts: 1,
  maxConcurrentLogins: 2,
},
pro: {
  maxCustomPacks: 5,
  // ...
},
Enter fullscreen mode Exit fullscreen mode

Read the trial line again. The comment says "no custom packs on the trial". The value says unlimited. The call site skips the check entirely when the value is null.

A free trial account could create as many custom question packs as it liked, while a paying Pro customer was capped at five.

Why nobody caught it

This is not a typo you spot in review, because everything about it looks right.

The value null is the correct type. TypeScript is completely satisfied. null is in the union, the call sites handle null, the exhaustiveness is fine.

The comment is in the right place and says the right thing about the intent. It is just describing a different value than the one on the line.

And the failure is silent in the direction nobody tests. Product people test that limits block things. Nobody writes a test asserting that a trial user gets refused at pack number one, because "trial users have no custom packs" sounds like a statement about the UI, not about a counter.

The one line fix is to write the sentinel that the contract actually defines:

// 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.
maxCustomPacks: 0,
Enter fullscreen mode Exit fullscreen mode

The fix that would have prevented it

number | null overloads one field to carry two different kinds of information: a quantity, and a mode. The bug is that "zero of them" and "no ceiling on them" are opposite meanings expressed in the same slot, and one of them is a value you could plausibly type by accident.

If you want the compiler to help, make the mode a separate thing it has to look at:

type Limit =
  | { kind: 'capped'; max: number }
  | { kind: 'unlimited' }

const LIMITS: Record<PlanType, { customPacks: Limit }> = {
  trial:    { customPacks: { kind: 'capped', max: 0 } },
  pro:      { customPacks: { kind: 'capped', max: 5 } },
  ultimate: { customPacks: { kind: 'unlimited' } },
}

function isOverLimit(limit: Limit, current: number): boolean {
  return limit.kind === 'capped' && current >= limit.max
}
Enter fullscreen mode Exit fullscreen mode

Now "none at all" is { kind: 'capped', max: 0 } and it is not reachable by leaving a field out or defaulting it wrong. More importantly, isOverLimit is one function that every call site uses, so the "skip the check when unlimited" logic exists once instead of being retyped at each of the four places that enforce a limit.

That is the real lesson, and it is not about null. It was not the sentinel that made the bug invisible, it was that the interpretation of the sentinel lived at every call site rather than in one function next to the data.

We did not do the full refactor, because six fields across three plans is not where that abstraction pays for itself yet, and a wrong abstraction costs more than the right sentinel. We changed the value and wrote the reason into the comment so the next person who is tempted to "tidy" that 0 back to a null has to read why.

A comment is not a test, and a pricing page is not decoration

The thing that finally made this findable is worth more than the fix.

The numbers in that table are the same numbers we publish. Up to five quizzes a day on Pro, up to fifty tables, up to five custom packs. Unlimited on Ultimate. Ten tables and one session on the trial.

That makes the pricing page a readable specification of the limits table, written in a language a non engineer can check. Go and look at it: the "What's included" list under each plan is the enforcement table with the field names taken out. Every one of those bullets is a row in the LIMITS record, and if a bullet and a row disagree, one of them is a bug and a customer will find out before you do.

If you have a limits table in your codebase, open your own pricing page beside it and read them line by line. It takes four minutes. We found one.

You can also just try it: the trial needs no card, includes a complete session with all the built in question packs and up to ten tables, and will now correctly refuse to let you create a custom pack.

Top comments (0)