The bugs that don't page anyone

An exception is a gift. It has a stack trace, it lands in the log, it turns a request red in every dashboard you own. Somebody fixes it by lunchtime.

The bugs that cost me the most this year in a multi-module ASP.NET Core MVC application — raw ADO.NET over SQL Server stored procedures, server-rendered grids, a little JavaScript — threw nothing. They returned 200 OK with an answer that looked right. A list that ignored its filter. A page that confirmed an email that was never sent. A grid column that was configured, mapped and visible, and empty.

Looking back over the fixes, every one of them was the same move: find where the system was silent about being wrong, and make it loud. A compiler error. A test. An error severity that actually reaches the caller. Here they are, roughly in order of how long each one survived.

1. The model binder changes the rules without telling you

This one is in the framework, and it is nasty precisely because it is documented.

When an action parameter has the same name as a property of its own type, ASP.NET Core switches that parameter to prefixed model binding. Instead of looking for Status in the form, it looks for filter.Status. The form doesn't send that, so the DTO arrives with every property null:

public sealed class OrderFilter
{
    public string? Filter { get; set; }   // free-text search box
    public string? Status { get; set; }
    public DateTime? From { get; set; }
}

// BAD: parameter "filter" + property "Filter" on its own type -> prefixed binding.
// Every property arrives null; the query runs unfiltered.
[HttpPost]
public IActionResult IndexPost(OrderFilter filter) { ... }

// GOOD: rename the parameter. Never suppress the warning.
[HttpPost]
public IActionResult IndexPost(OrderFilter criteria) { ... }

No exception, no validation error. The user picks a status, presses Search, and gets back the full, unfiltered list — which, on a busy table, looks exactly like a list that happens to contain mostly matching rows. It was in production before anyone noticed.

The analyzer does catch it: warning MVC1004. The problem was that it was one warning among roughly 720 in the build output, and a warning nobody reads is not a signal. The fix that stays fixed is in the project file:

<!-- An action parameter named like a property of its own type switches binding
     to prefixed mode: the DTO arrives all-null and filters silently stop
     applying. Invisible among hundreds of warnings, so it is an error. -->
<WarningsAsErrors>$(WarningsAsErrors);MVC1004</WarningsAsErrors>

I'd add that line to every MVC project template I own. The broader lesson is about the other 719: if your build has enough warnings that a correctness warning can hide in them, pick the ones that mean "wrong result" and promote them, even if you can't clean up the rest yet.

2. The redirect loop made of two different counts

Our list pages follow Post/Redirect/Get: the filter form POSTs, the criteria go into TempData, and the action redirects to Index. Index has a guard: if the requested page is past the end, go back to page 1. Sensible. It looked like this:

// BAD
if (page > p.TotalPages && p.TotalRows > 0)
    return RedirectToAction("Index", new { page = 1 });

TotalPages came from the filtered count. TotalRows was the unfiltered total — "is there any data at all?". Now apply a filter that matches nothing:

page (1)  > TotalPages (0)   ->  true
TotalRows (500) > 0          ->  true

Redirect to page 1. The filter is still in TempData, so the next Index computes zero filtered rows again, and 1 > 0 is still true. Redirect. Forever. The browser spins on 302s until it gives up with a "too many redirects" page that tells the user nothing. It didn't matter which page you started on — only that the result set was empty, which is the single most normal outcome of a search.

The fix is to guard on the same number you paginate with:

// GOOD: redirect only when a filtered result set with at least one page exists
if (p.TotalPages > 0 && page > p.TotalPages)
    return RedirectToAction("Index", new { page = 1 });

The pattern had been copied into about five controllers, so the fix came with a written spec and a migration table listing each one. And while auditing them, a worse variant turned up. One controller used an unfiltered count procedure for both the guard and the pagination shown to the user. Separately, its list procedure accepted the filter parameter and ignored it, and the count procedure didn't take one at all. The result: filters did nothing, the pager showed unfiltered totals, and everything returned 200.

Two rules came out of this:

  • Compute the guard and the displayed metadata from the same query. If two numbers describe the same page, they must come from the same place.
  • Test the empty result. It is the case every developer skips in a manual check, because when you test a search you search for something that exists.

3. A column has to exist four times

The application's grids are metadata-driven: a configuration table says which columns are visible, in what order, and whether they appear in the spreadsheet export. That's flexible, and it hides a trap. For a column to actually show a value, it has to exist in four places:

  1. a metadata row marked visible,
  2. the stored procedure's projection,
  3. the controller's row mapper,
  4. a model property the view reads.

Miss any one and the result is identical: an empty cell, no error. We had model properties that existed, columns that were configured visible, a procedure that returned them — and a mapper that had simply never been updated to copy them. The configuration was correct, which is exactly why it took a while to look at the mapper.

A few cousins of the same bug:

  • Export is a separate flag. A column can be visible in the grid and missing from the spreadsheet, and users find out only when they open the export.
  • Header and body are built separately. The header comes from the metadata's ordinal positions and an actions column is appended at a fixed place; the body appends its actions cell last. Add a column after the actions column in the metadata and every header shifts one cell away from its data.
  • DataTables passes undefined, not null. On an empty table, the columnDefs render callbacks receive data === undefined. A renderer that guards against null and then calls data.indexOf(...) throws, and the table never initialises — on the empty-result page again.

The mitigation for schema drift between environments was a small helper that reads an optional column only if it is present in the result set. A page deployed ahead of its database patch then shows an empty column instead of throwing — which, for once, is the silent behaviour you want, because it is deliberate and temporary.

4. An error raised at a volume nobody hears

A user-facing flow sends a one-time link by email. The C# side calls a mail-sending stored procedure inside a try/catch, and shows an error if it fails. It never showed an error. Users were told the email was on its way, and waited for a link that never came.

The procedure caught the mail subsystem's errors and re-raised them like this:

RAISERROR(@msg, 10, 1);

Severity 10 is informational. In SQL Server, severities of 10 and below are returned as messages, not errors; SqlClient surfaces them through the connection's InfoMessage event, and they never become a SqlException. The C# catch was dead code. It had never run.

The fix is a one-digit change:

RAISERROR(@msg, 16, 1);

16 is the conventional "user-correctable error" severity, and it surfaces as an exception, so the existing error path started working exactly as designed. It needed a DBA to apply it — and until that happened, the bug was tracked as open, not as "fixed in the repo". A fix that only exists in a migration script hasn't fixed anything yet.

The more general habit: in a stored-procedure codebase, error numbers and severities are part of the API. Ours now maps specific THROW numbers to specific HTTP responses (a conflict becomes a 409, a validation failure a 400), and I review a severity in a procedure as carefully as a status code in a controller.

5. Defensive code that only moves the failure

The same flow had a front-end problem. The script read the anti-forgery token like this:

// BAD
var token = (document.querySelector('meta[name="csrf-token"]') || {}).getAttribute('content') || '';

The layout's meta tag had a different name, so querySelector returned null. The || {} was presumably meant to make that harmless. It doesn't: an empty object has no getAttribute, so the line throws a TypeError — one that talks about getAttribute, not about a missing meta tag. The fallback didn't prevent the failure; it just moved it somewhere less informative. The same selector had been copied into two older flows in another module, which had quietly never worked.

And once the selector was fixed, a second layer appeared: on those pages the meta tag existed but was empty, because the anti-forgery token was only rendered into the layout when the action explicitly generated it. An empty token posts fine — and [ValidateAntiForgeryToken] answers every request with 400. Each GET on those pages now generates the token.

The rule I took from it: don't wrap something that must exist in a fallback. If the element is required, look it up and fail loudly, at the point of the lookup, with a message that names it:

var meta = document.querySelector('meta[name="app-antiforgery-token"]');
if (!meta || !meta.content) throw new Error('anti-forgery meta tag missing or empty');
var token = meta.content;

6. A column width that rolled back the business write

The last one is small and very SQL Server. An audit-log row was written in the same transaction as the business change it described. The log's username column was nvarchar(20). A domain-qualified username — DOMAIN\firstname.lastname — is longer than 20 characters for a lot of people.

Insert the log row, hit the width, and the whole transaction rolls back: the user's actual change is gone, because the log of it didn't fit.

We widened the column, and the service now validates field lengths explicitly and rejects over-long input with a clear 400 rather than truncating silently. Truncation is just another way of storing a plausible wrong value.

Making the silence loud, permanently

Fixing each bug was easy once it was found. The work worth describing is what stops the class from coming back.

Analyzer warnings as errors, for the ones that mean "wrong result" — MVC1004 first.

Reflection tests for invariants nobody wants to re-check by hand. A small xUnit class walks the MVC assembly and fails the build if a data endpoint loses its authorization filter, if a debug endpoint that was deliberately removed comes back, or if [AllowAnonymous] appears anywhere outside a short whitelist:

[Fact]
public void AllowAnonymous_OnlyOnKnownActions()
{
    var allowed = new HashSet<string>
    {
        "HomeController.Error",
        "AccountController.Login",
        "AccountController.AccessDenied",
    };

    var anonymous = typeof(HomeController).Assembly.GetTypes()
        .Where(t => typeof(ControllerBase).IsAssignableFrom(t) && !t.IsAbstract)
        .SelectMany(t => t.GetMethods(BindingFlags.Public | BindingFlags.Instance | BindingFlags.DeclaredOnly)
            .Where(m => m.GetCustomAttribute<AllowAnonymousAttribute>() != null)
            .Select(m => $"{t.Name}.{m.Name}"))
        .ToList();

    anonymous.Should().OnlyContain(name => allowed.Contains(name));
}

These tests are cheap, fast, and catch exactly the regression that a code review misses because the diff that causes it looks harmless.

A smoke matrix with the boring rows in it. Every go-live checklist now has explicit rows for zero results, missing permission and upstream unavailable. Those are the rows where the bugs above lived, and they are the rows nobody tests by instinct.

Over the period these fixes landed, the test suite went from 78 tests to 336. Very few of them test anything clever. Most of them pin down a place where the system used to be quiet.

Checklist

  • Promote correctness analyzer warnings to errors — start with MVC1004
  • Never name an action parameter after a property of its own type
  • Compute pagination guards and pagination metadata from the same filtered count
  • Test the empty result set, on every list page
  • For metadata-driven grids, check all four layers: metadata, projection, mapper, model
  • Guard DataTables renderers against undefined, not just null
  • Raise database errors at severity 11 or above (16 by convention); 10 is a message, not an error
  • Treat THROW numbers and severities as API, mapped to HTTP status codes
  • Don't put || {} around something that must exist — fail at the lookup, with a useful message
  • Validate field lengths before a write transaction; don't let a log row roll back the business row
  • Add reflection tests for authorization and exposure invariants

Code samples are simplified and renamed from a production ASP.NET Core MVC application (raw ADO.NET, SQL Server stored procedures, DataTables) on .NET 8.