Our question banks are 226 JSON files, about 14MB, sitting in lib/games/questions/banks/. They are read off disk at request time rather than imported, for a reason the reader's own docstring states:
/**
* Server-side reader for the authored question banks.
*
* The banks are plain JSON files under `lib/games/questions/banks/`. They sit in
* `lib/` rather than `public/` deliberately: anything under `public/` is served
* by the CDN to anyone who knows the filename, and these files contain every
* answer key in the product.
*/
I have written before about how they ended up on the CDN once and about the import graph being the only thing keeping the keys off your machine. I have also written about the deployment consequence: Next traces your imports, so files you read with fs are not deployed unless you say so in next.config.mjs.
This post is the sequel to that last one, and it is about the shape of the config entry rather than its existence.
The entry names a route, and nothing else
outputFileTracingIncludes: {
'/api/games/questions/[bankId]': ['./lib/games/questions/banks/**/*.json'],
},
That is a map from route to globs. It is not a map from module, from function, or from anything the type checker can see. The route is a string that has to match a path in your app, written by hand, in a file that is not type-checked against your routes.
For a long while that was fine, because exactly one thing read the banks. Then we shipped the answer review on session reports, which pairs what you answered with the question you answered it on, and that needs the bank.
Four steps from the key to the read
The new reader is not a route. Here is the whole chain:
-
app/api/feedback/[sessionId]/route.ts, which is what the config would have to name - imports
lib/games/review/build-review.ts - which imports
lib/games/questions/bank-source.ts - whose
readQuestionBankcallsreadFile
The config key lives at step 1. The filesystem access lives at step 4. Nothing in steps 2, 3 or 4 can tell you which routes they are reachable from, and nothing at step 1 mentions a file.
So the fix is one line, and the comment above it is longer than the change:
// The question banks under `lib/games/questions/banks/` are read off disk at
// request time, not imported, so Next's dependency tracing cannot see them.
// Every route that calls `readQuestionBank` needs an entry here: without one
// the route deploys without its data and the read fails in production while
// working perfectly in `next dev`. `app/api/games/questions/[bankId]` serves
// the banks to games; `app/api/feedback/[sessionId]` builds the report's
// answer review from them. Declared as a glob so a newly authored bank is
// included the day it lands, with no config change.
outputFileTracingIncludes: {
'/api/games/questions/[bankId]': ['./lib/games/questions/banks/**/*.json'],
'/api/feedback/[sessionId]': ['./lib/games/questions/banks/**/*.json'],
},
The old comment named the one route as if naming it were the point. The new one states the rule first (every route that calls readQuestionBank needs an entry) and then names the two routes that currently qualify. That reads like a pedantic edit. It is the difference between a comment that documents the past and a comment that can be checked against the code.
Why this bug is quiet rather than loud
The missing-files failure mode is the worst kind of good behaviour. In next dev the repository is on disk, so every read works and nothing hints that a config entry is needed. In production the serverless function is built from the traced file list, the JSON is simply not there, and readFile rejects.
And on this particular route, the review is deliberately wrapped so that its failure cannot take the report down:
// The answer review is built here rather than in the browser so the page
// gets only the questions this session answered, not the bank's keys. A
// failure costs the review, never the report.
let review: AnswerReview | null = null;
try {
review = await buildAnswerReview(
session.game_id,
session.responses as SessionResponses | null
);
} catch (reviewError) {
logError(`Answer review failed for session ${session.id}`, reviewError);
}
That is the right call for a user looking at their score. It also means the production symptom of a missing config entry is not an error page. It is a report that silently has one fewer section than the same report on your laptop, which you will notice on the day you happen to compare them.
Two environments, one of which has your whole repository lying around, is a category of bug worth naming out loud: anything that resolves a path at runtime is untested by local development.
What would actually guard it
A comment is a guard that works exactly as long as people read it. The real check is mechanical and not hard to describe: walk the import graph from every file under app/api/**/route.ts, and if lib/games/questions/bank-source is reachable, assert the route's path is a key in outputFileTracingIncludes.
We have not written it. We do write a lot of tests that assert our content is true rather than our code, so the appetite is there, and this is the same genre: an assertion about a relationship between two files that no compiler models. For now the honest statement of the situation is that the rule is in a comment, the rule has two instances, and the day there is a third the comment is where the author will look.
One detail that did go right: the value is a glob, not a list of filenames. We author banks continuously, and a config file that had to name 226 JSON files would be wrong within a week.
The parse that moved
A smaller thing I liked. The original reader returns raw text, and says why:
Returns the text rather than a parsed object because the route hands it straight to the client: parsing it here only to re-serialise it would cost CPU on every request for a payload nothing on the server inspects.
The review is the first server-side consumer that does inspect it, so it parses:
const { questions } = JSON.parse(await readQuestionBank(gameId)) as { questions: unknown[] };
The docstring is still correct about the original caller, and the new caller pays the parse cost itself rather than imposing it on the route that does not need it. That is the version of a shared helper I want: the cheap thing by default, and the expensive thing at the call site that asked for it.
The pages behind the banks
The banks that this particular route reads are the ones behind the eight UK Civil Service online tests, which is where the answer review shipped first. The same /api/games/questions/[bankId] route serves every bank behind the 56 providers on the games page, and the test format pages are the non-branded way in if you do not yet know which provider your employer uses.
Top comments (0)