Invental/Review lessons/Code that silently does the wrong thing

21 lessons · 31 checks

Code that silently does the wrong thing

The worst bugs return 200. A schema drops a field, a falsy zero becomes "not set", a loading state is read as "not found". Nothing throws, so nothing alerts. These lessons cover where that happens and how to make it loud.

Major

An ORM default does not change the database

Changing a default in your ORM schema changes your types, not your database; and a default that pins a fast-moving build tool is a product decision, not a version bump.

Read the lesson → Major

Framework internals can break error handling

Importing a framework's internal helpers can silently break its error handling: an instanceof check fails when the class comes from a second bundled copy, and every typed error becomes a 500.

Read the lesson → Major

Map not-found to 404 around one lookup

Translate a database “not found” into a 404 only around the one query whose absence means “not found”. If the catch wraps a whole flow, a missing row in a later query gets reported as the wrong error.

Read the lesson → Major

Copy a tool’s credential lookup order exactly

When you re-implement another tool's lookup rules (credentials, config files, env precedence), copy its order and its failure behaviour exactly, read from its source, or users get different credentials in your tool than in theirs.

Read the lesson → Major

Sorting each batch is not a global order

Sorting inside each batch is not the same as ordering globally: if a priority has to win over a dependency graph, pick the next batch by priority first, then sort.

Read the lesson → Major

A catch that only logs lets failures through

When you move error handling into an inner function, check the outer catch: if it only logs, the failure stops propagating and everything after it runs as if nothing happened.

Read the lesson → Major

A bare ?dryRun flag must mean on

Parse safety flags so that their presence means “on”. Then ?dryRun with no value does a preview and never writes.

Read the lesson → Major

Map API field names at the boundary

When the public API names differ from the internal ones, a straight pass-through can validate cleanly and still drop the data; map the names explicitly at the boundary and test that the write took effect, not just that the call succeeded.

Read the lesson → Minor

A skipped check is not a passed check

“Could not check” is not “checked and fine”: return a third state for skipped AI or validation calls so nothing caches it as a pass.

Read the lesson → Major

Schemas that strip unknown keys hide bugs

A schema that strips unknown keys turns “forgot to add the field” into silent success; when you add a field to an API, check that the parse schema, not just the handler, knows about it.

Read the lesson → Minor

CI directory defaults skip action steps

CI defaults are narrower than they look: GitHub Actions' defaults.run.working-directory covers shell steps only, so every uses: step in a sub-directory job needs the directory passed explicitly.

Read the lesson → Major

A timestamp alone is not a paging cursor

A pagination cursor must be unique and self-contained: pair the timestamp with the row id, and store the values you compare against rather than a pointer to a row that might be deleted.

Read the lesson → Major

Scope a kill switch to the feature it guards

A kill switch added to a shared validation path must be scoped to the feature it guards, or flipping it silently disables checks for unrelated data that shares the same code.

Read the lesson → Major

Audit stored values before reusing a field

Before giving a stored field a new meaning, list every value it already holds in production and who wrote it; a side-effect '' or a legacy false will be read by the new code as a deliberate choice.

Read the lesson → Major

An LLM that rewrites a file must see all of it

When a model is asked to output a whole file that you then write to disk, it must have seen the whole file; truncating the input silently turns an “edit” into a deletion.

Read the lesson → Minor

Paging results merged from several queries

When one “page” is the merge of several paged queries, page every query with the same offset. “Is there more?” is true if any source has more. Paging only the main query silently drops the rest.

Read the lesson → Major

enabled: false does not stop refetch()

enabled: false only pauses automatic fetching: a manual refetch() still fires, and old cached data stays visible. To make a query truly impossible, swap the query function for skipToken, and decide separately what cached data should show.

Read the lesson → Major

A read-only backup role is not one grant

A “read-only backup role” in Postgres is not one grant: pg_read_all_data skips large objects, table owners skip RLS unless it is forced, and --schema dumps silently leave large objects out.

Read the lesson → Major

Still loading is not the same as not found

When you swap a synchronous mock for a real API, every “if there's no data, treat it as missing” check becomes a bug, because “still loading” also has no data. Mocks that answer instantly hide this whole class.

Read the lesson → Minor

When 0 is valid, decide option precedence

When 0 is a valid value for an option, decide precedence explicitly (“if set, ignore the other option”) instead of patching truthiness checks one at a time.

Read the lesson → Major

Bulk actions must act on the whole selection

A bulk-selection UI must not offer a single-record action for a multi-record selection: the command either loops over every selected record or is hidden. And fix render churn where the unstable value is created, not in the shared hook downstream.

Read the lesson →

Checklist: 10 ways your app says “success” and does nothing →

The checks, with the lesson behind each

When you change a column default in your ORM schema, do you ship the migration that changes it in the database too?+
Changing a default in your ORM schema changes your types, not your database; and a default that pins a fast-moving build tool is a product decision, not a version bump.
Do your webhook endpoints still return 400 for bad JSON and 413 for oversized bodies on a production build?+
Importing a framework's internal helpers can silently break its error handling: an instanceof check fails when the class comes from a second bundled copy, and every typed error becomes a 500.
Does each 404 in your API come from one specific lookup, not from a catch around the whole request?+
Translate a database "not found" into a 404 only around the one query whose absence means "not found". If the catch wraps a whole flow, a missing row in a later query gets reported as the wrong error.
If your tool reads Docker or cloud credentials, does it pick the same credentials, in the same order, as the official CLI on the same machine?+
When you re-implement another tool's lookup rules (credentials, config files, env precedence), copy its order and its failure behaviour exactly, read from its source, or users get different credentials in your tool than in theirs.
For any "force this order" setting in your system, have you tested it with a dependency that pulls something forward?+
Sorting inside each batch is not the same as ordering globally: if a priority has to win over a dependency graph, pick the next batch by priority first, then sort.
In your error-handling refactors, does every catch that reports a failure also rethrow or return an error?+
When you move error handling into an inner function, check the outer catch: if it only logs, the failure stops propagating and everything after it runs as if nothing happened.
If someone calls your destructive endpoint with ?dryRun and no value, does it write anything?+
Parse safety flags so that their presence means "on". Then ?dryRun with no value does a preview and never writes.
For each update endpoint, does a test read the record back and assert the new value, not just a 200 response?+
When the public API names differ from the internal ones, a straight pass-through can validate cleanly and still drop the data; map the names explicitly at the boundary and test that the write took effect, not just that the call succeeded.
When your AI or fraud check is skipped (quota, timeout, rate limit), does the caller know it was skipped, or does it look like a pass?+
"Could not check" is not "checked and fine": return a third state for skipped AI or validation calls so nothing caches it as a pass.
For the last field you added to an endpoint, is there a test that sends it and asserts it was stored?+
A schema that strips unknown keys turns "forgot to add the field" into silent success; when you add a field to an API, check that the parse schema, not just the handler, knows about it.
In your CI, does every action step in a sub-directory job get the directory explicitly, or do you rely on a run default that it ignores?+
CI defaults are narrower than they look: GitHub Actions' defaults.run.working-directory covers shell steps only, so every uses: step in a sub-directory job needs the directory passed explicitly.
Does your "load newer" or sync endpoint page by timestamp alone?+
A pagination cursor must be unique and self-contained: pair the timestamp with the row id, and store the values you compare against rather than a pointer to a row that might be deleted.
When you add an "off switch" to a shared validator, is there a test proving that other record types are still validated with the switch on?+
A kill switch added to a shared validation path must be scoped to the feature it guards, or flipping it silently disables checks for unrelated data that shares the same code.
For the field you're about to reuse, have you queried what values existing rows hold and which old code path wrote each?+
Before giving a stored field a new meaning, list every value it already holds in production and who wrote it; a side-effect '' or a legacy false will be read by the new code as a deliberate choice.
In any job where an LLM rewrites a file, can the input ever be shorter than the file it replaces?+
When a model is asked to output a whole file that you then write to disk, it must have seen the whole file; truncating the input silently turns an "edit" into a deletion.
If your search merges results from several queries, can page 2 still show a match that only the second or third query finds?+
When one "page" is the merge of several paged queries, page every query with the same offset. "Is there more?" is true if any source has more. Paging only the main query silently drops the rest.
For a feature that must never call an API in some state, does a manual refresh button also respect that rule?+
enabled: false only pauses automatic fetching: a manual refetch() still fires, and old cached data stays visible. To make a query truly impossible, swap the query function for skipToken, and decide separately what cached data should show.
Has anyone restored your "read-only" backup end to end, including large objects, using the exact role and flags in your runbook?+
A "read-only backup role" in Postgres is not one grant: pg_read_all_data skips large objects, table owners skip RLS unless it is forced, and --schema dumps silently leave large objects out.
Does any screen redirect, toast "not found" or close a panel based on missing data without checking that the request has finished?+
When you swap a synchronous mock for a real API, every "if there's no data, treat it as missing" check becomes a bug, because "still loading" also has no data. Mocks that answer instantly hide this whole class.
Do any of your API options treat 0 or an empty string as "not provided"?+
When 0 is a valid value for an option, decide precedence explicitly ("if set, ignore the other option") instead of patching truthiness checks one at a time.
Pick two rows in your app's table and click each bulk action: does every one act on both rows or refuse?+
A bulk-selection UI must not offer a single-record action for a multi-record selection: the command either loops over every selected record or is hidden. And fix render churn where the unstable value is created, not in the shared hook downstream.

More checks from the same reviews

Yes/no questions from other reviews on this theme. Each one is quick to answer for your own app.

  1. Does every indexOf / findIndex result in your code get checked for -1 before it is used in arithmetic?
  2. When a query moves into a repository, does the “not found” case still return the same status code it did before?
  3. Do any of your “not equal” filters run on a nullable column where NULL should count as “different”?
  4. Do your 404 responses for “not created yet” resources say Cache-Control: no-store?
  5. If local storage throws when a visitor rejects cookies, does your next page load still treat them as having accepted?
  6. In your config or SDK, if a user sets the same option twice, do they get an error or a silent winner?
  7. If your database or storage is down, does your app show users an error, or an empty screen that looks like lost data?
  8. When your AI response stream fails halfway, does your app save the half answer as if it were complete?
  9. For every third-party list call your app makes, do you know what happens when the account has more items than one page?
  10. Run docker volume ls on a dev machine. Is there a volume literally named “undefined” or “null”?

Want a senior reviewer to run these checks on your code? Is your vibe-coded MVP production-ready? →

Want these checked on your code?

We run reviews like these on real codebases. Tell us what you are building and we confirm scope before anything starts.

— Get in touch
hi@invental.co ↗
— Or
— What to include

A link or short description of the code or system, what you want checked, your stack, and when you need the answer. No repository access is needed until scope is agreed.

— Or leave a note
We reply within one business day.