Condition handling: discriminate, recover, never mask

The five rules above are written in the vocabulary of handlers, and I scoped them too narrowly. What I am prohibiting is destroying a cause that was available. A handler clause is one place that happens, and it is not the interesting one, because it is the place everybody already watches.

Here is the same violation with no handler anywhere in it:

(multiple-value-bind (out err code) (ssh-exec node ...)
  (declare (ignore err code))      ; the exit status is discarded HERE
  ... (or out "") ...)             ; and a plain value goes out in its place

That is (handler-case … (error () nil)) written in return values. The cause was present, it was named, and it was thrown away. No handler-case appears, so nothing here was looking at it: not this document, and not no-ignore-errors, which fires on a spelling and would not have seen this shape even if one had.

The cost was not hypothetical. A predicate in proving-ground/driver/liveness.lisp shelled out and reduced the result to a boolean, so it answered NIL for a thing that was genuinely absent and NIL for an ssh that never connected. Under refutation a NIC was reported moved out of a host nobody had reached.

So read every rule above as governing the cause, wherever it is discarded. An exit status, an error string returned beside a value, a nil standing in for an answer that was never obtained: these are the same defect as a bare handler, and they are harder to see precisely because no keyword marks them.

⛔ Do not answer this with a text scan for the spelling. Whether a cause survived is a property of meaning, and a guard written in the vocabulary of spellings catches only the spelling you already thought of. I wrote four candidate predicates for the blanket-handler shape and a real site in this tree refuted every one, because the channel by which a cause reaches its reader is unbounded: re-signalling, a returned value carrying the condition, even a condition handed across a thread boundary in a mailbox. Keep a scan if you like, as a tripwire that is never counted as the guarantee.

The protection that works is the one rule 1 of the next section already names, so apply it here rather than reading it as being about data alone: I made the reading itself carry whether it was taken. observe-on-guest answers an observation that holds the refusal when the guest was never reached, and transport-refusal is the single place a status becomes a type. A predicate built on that inherits the refusal instead of reimplementing the check, and a question asked of a reading that never happened cannot quietly answer off an empty string.

⚠ Where a typed condition for the event already exists, converge on it rather than inventing a second vocabulary beside it. Two encodings of one event drift, and the drift is invisible at the seam where they meet.

⛔ Do not flatten a value to a common representation in order to make an error go away. Coercing everything to simple-string, or storing everything in a text column, removes the symptom and the information together. What is lost is the domain distinction, and the cost is paid later and elsewhere: every consumer recovers the distinction by inspecting characters, and those inspections disagree.

This is the data-shaped form of the failure this document exists to prevent. A blanket handler passes because it cannot tell cases apart. A flattened type passes for exactly the same reason.

At a foreign boundary that genuinely demands a representation, and there only. A library that accepts only a simple-string is someone else's requirement, and adapting to it belongs at that call, narrow and named, not upstream where it would spread the assumption through code that had no such requirement. One question settles it: is this coercion serving a real external constraint, or is it making an error message stop?

A DNS NOTIFY recipient is legitimately either a name or an address. It was stored as text, with a column-name convention and a runtime audit rule carrying the type information the schema did not. The value arrived as a non-simple string, passed unchanged through a normaliser (string-trim returns its argument eq when there is nothing to trim), passed through a codec that is representation-agnostic, and was refused at the socket boundary by a foreign function that accepts only a simple string.

Three layers handled the value and only the last had an opinion about its type. The feature was built, wired, armed, and had never once succeeded.

⚠ The detail that makes the case: the one recipient shape that came through clean was the dotted IP literal, because that was the only path that happened to run subseq. The ordinary hostname carried the hazard. That is what discrimination-by-string-inspection buys, and coercing the recipient upstream would have made every shape uniformly clean and uniformly untyped, hiding the finding rather than answering it.

The correct fix was one named adapter at the foreign call that had the requirement, with nothing above it changed.

A cleanup form is the one place ignore-errors earns its keep:

(unwind-protect
     (serve-session session)
  (ignore-errors (close-session session)))   ; legitimate

The reasoning is specific, and it does not generalise. During an unwind there is no caller left to act on a secondary failure, and worse, a condition signalled from a cleanup form displaces the original condition, replacing the failure you need to diagnose with a less informative one from the teardown path. Closing an already-dead socket is the common case: it fails precisely because the thing you actually want reported has already gone wrong.

So the test is position, not shape. (ignore-errors (close x)) in a cleanup form is correct. The same expression in a protected form is masking: it discards a distinction someone still could have used.

Of the 115 sites frozen in the baseline, 85 are cleanup-position closes of exactly this kind, about three quarters. Those are not debt and should not be rewritten into handler-case; they should be suppressed at the site with the reason stated, and they do not count toward the retirement of the file. Retiring a listed file means resolving the masking sites, not the cleanup ones. The real masking debt is 16.

A cleanup-position ignore-errors wrapped around a call that cannot succeed is not cleanup. It is a permanently broken teardown wearing cleanup's clothing, and it is invisible precisely because the masking looks legitimate.

valis shipped one, and I am keeping it here after the repair because the shape is the lesson. The resident's failed-push cleanup, in %bring-up-declaring-socket (src/main.lisp), read (ignore-errors (close listen-fd)) in an error path: textbook cleanup position. But listen-fd is a bare integer descriptor dup'd out of an iolib socket, the file is (:use #:cl), so that was cl:close, and (close 7) signals a type-error. The call never once succeeded, the descriptor was never released, and the ignore-errors was hiding its own cleanup failing.

The repair gave the release a name of its own. close-listening-fd (src/backends/epoll.lisp) is the exact counterpart of make-listening-fd: it issues the close(2) syscall on the bare integer, and it keeps iolib confined to the backend, so a consumer that mints a LISTEN descriptor can release one without importing iolib and without shadowing close in its own package. The cleanup site now calls that, and the mask over it is a genuine cleanup mask, suppressed at the site with the reason stated: a close error there must not displace the push error being re-signalled.

So add a pass to the triage: for every site classified as cleanup, check that the cleanup call can actually succeed. Match the argument's type against what the function accepts. This is where descriptor- and handle-heavy code needs the most scrutiny, and it is the one check a mechanical position rule will skip.

And the callee must be knowable. The test is three-part, not two: cleanup position, a cleanup call that can succeed, and a checkable callee. Consider

(when teardown (ignore-errors (funcall teardown)))   ; in a cleanup form

Position clears it. "Can the call succeed?" has no answer: teardown is an opaque closure supplied by the caller, so the question resolves to nothing and any bug anywhere inside any teardown closure is swallowed permanently and invisibly. A cleanup-position mask over funcall, apply, an open generic dispatch, or any caller-supplied hook is the shape where both earlier tests pass and the mask is still unbounded. Narrow what such a site catches; do not bless it.

Verifying a masked call requires calling it unmasked. A passing test suite proves nothing about a masked cleanup: an always-signalling call inside ignore-errors leaves the suite green, which is exactly the invisibility that hid the resident's broken descriptor release. To establish that a masked cleanup call works, drive it bare, outside the mask, against a live object and observe that it returns.

That cuts deeper than it first appears. A mask does not only hide a defect that exists; it hides the absence of a guard against one that has not happened yet. If the teardown behind a mask were to break tomorrow (through a package change, a dependency upgrade, a refactor), no test would notice, because the mask swallows the breakage. So the question for a cleanup site is not only "does this work today" but "would anything fail if it stopped working." Where the answer is no, the guard is missing regardless of whether the code is currently correct.

no-ignore-errors skips defmacro outright. The whole form is skipped (the expander's own body and the expansion template alike), so a cleanup mask written anywhere inside a defmacro is neither reported nor suppressible. It is simply unenforced, and the gate's count will disagree with a plain grep by exactly those sites.

State the coverage as a negative: everything except defmacro is descended into. A positive list invites drift: defmethod (qualified or not), defgeneric, define-compiler-macro, macrolet, labels, flet and bare top-level forms are all checked, and any list of them written here will be missing one. There is a single skip branch, so the negative form cannot develop a hole the way an enumeration can.

Three properties of that skip are easy to get wrong:

• Backquote is not the trigger. A backquote inside a defun is checked normally. The reader yields quasiquote structure, not cl:quote, so nothing about backquote reaches the skip: a defmacro with no backquote at all is skipped just the same.
• The skip is per-node, not top-level. A defmacro nested inside a defun, a progn or an eval-when is skipped identically, so "my sources have no top-level defmacro" does not establish coverage. Check unanchored.
• quote is a second skip category. A masking form inside '(…) is not examined either. Defensible (that is data, not code), but it is a second way a site goes unseen.

The macro hole is the smaller one. no-ignore-errors fires on exactly one head: ignore-errors. It never examines handler-case at all, in any form, macro or not. So (handler-case x (error () nil)) is not partially gated or heuristically gated: it is outside the rule entirely, and no configuration reaches it. Every count this gate reports is a count of a spelling, not of the defect; and the rule's own message points a reader at the ungated form. Closing that needs a new rule, not a tuning of this one.

Scope, and the two halves differ. The defmacro skip is written into no-ignore-errors itself, so it says nothing about any other rule: each implements its own form handling, and a future rule written to catch the blanket handler-case will skip defmacro only if its author writes that branch again. The quote skip is different: it lives in the shared traversal guard, so it applies to every rule built on it. Do not generalise the first; do expect the second.

Cross-check your own tree, per file, never in aggregate. A whole-tree total hides the gap behind offsetting differences and reads as clean.

A delta is not a finding until every site in it is attributable. Three different things produce one, and only the first is a blind spot: a template site the gate cannot see, a site you suppressed at the site, and a site frozen by a path block. The latter two are your own configuration working correctly, and a naive reading of the delta reports them as a coverage hole. Take the measurement against a neutral configuration first (that separates the causes in one step), then account for every remaining site against a template you can point at. valis's 72 were confirmed this way: identical counts under a neutral config, and the masks in the shared test helper sit inside defmacro forms, which the gate skips whole. Those particular masks happen to sit in expansion templates, but that is incidental: the enclosing defmacro is what makes them invisible, and a mask in the expander's own body would be just as unseen.

In valis, src/ is fully covered: 115 counted directly, 115 reported, zero per-file delta. tests/ is not, and it is not close: 466 counted against 394 reported, 72 sites the gate cannot see, spread across 26 files. That has no practical effect today because tests/** is scoped off anyway, but the coverage claim has to be stated per tree rather than as one number.

Test scaffolding is where this concentrates, which is the worst place for it: tests/support.lisp reports 0 against 4, and a shared sealing-client helper 0 against 1. Setup and teardown wrapped in a with-… macro is exactly where masking accumulates and exactly where it is least often reviewed.

Two consequences worth stating plainly. First, planting a probe in ordinary top-level code proves the file is linted and says nothing about macro coverage. Plant one inside a defmacro if you intend to claim the latter. A probe inside a backquote does not test the skip: in a defun it reports like any other site, so it demonstrates only what the ordinary probe already did. Design the control against the thing that actually decides (the enclosing defmacro) and confirm it stays silent, with a second cell outside the macro confirming the rule fires at all. A silence you cannot distinguish from a rule that never ran is not evidence. Second, do not restructure a macro to make its cleanup visible to the linter. The code is correct as written; contorting it to satisfy a blind spot is the same error as stripping colons to move a count. Document the gap; do not engineer around it.

That resident cleanup site reached cl:close where a descriptor-specific close was meant, because the file is (:use #:cl) and nothing shadowed the symbol. Any domain verb colliding with a CL symbol has this exposure: open, close, read, write, listen, load, delete, merge, get, map, find, remove, replace, search, count, position.

Where a :shadowing-import-from is what makes the domain verb win, that single declaration is all that stands between correct behaviour and a type error landing next to cleanup masking, in files whose tests stay green. Ask the image what the symbol resolves to in each package rather than reading the defpackage and assuming.

The better question is the second-order one: what would have to change for that resolution to change? A shadowing import is one deleted line away from silently re-pointing every call in the file. Two shapes have no such line: fully qualifying the call at each site, and (where the semantics genuinely are "close this thing") extending the standard generic with a defmethod on cl:close rather than shadowing it. Then the domain behaviour is reached through the standard symbol, dispatch does the work, and there is no declaration whose removal could redirect it.

But that second shape has the opposite fragility, and it is the more dangerous one. Where a defmethod on cl:close is what makes an interposed behaviour work (a shield over a borrowed descriptor, say, whose close is deliberately a no-op so a layered teardown cannot close a socket it does not own), the method dispatches because the symbol is cl:close. Adding a (:shadow #:close) to that package silently defines the method on a new symbol. The interposition detaches, the caller's cl:close finds no applicable method, the default runs, and the borrowed descriptor is closed by code that had no right to close it.

So there are two directions, both of which look like package boilerplate and both of which are load-bearing:

Audit for both. "Which of my collisions depend on a shadow being present, and which depend on there being none?" A reviewer who has internalised only the first will wave the second straight through.

The frozen list is not one thing, and conflating its parts leads either to blind rewrites or to silent blessing:

1. Unexamined: not yet triaged. Freeze. The honest claim is "we have not looked at this."
2. Examined, not known defective: a masking shape that is behaviourally correct today. Freeze, with the diagnosis and intended fix recorded. The claim is "examined, not yet converted." Converting blind is worse than deferring: a fix prescribed by shape rather than by reading the site is how a correct rejection becomes a silent acceptance.
3. Examined, defective: a demonstrated bug. Fix it. Never freeze, never suppress. Separate commit, with a test that fails without it.

A conversion that can be made observably neutral should just be done. Where it cannot, freeze as category 2 and say so. Note that a semantics-preserving conversion does not get a "fails without the fix" test, and claiming one would misrepresent what changed: regression protection, labelled as such, is the correct artifact there.

%refresh-serving-index puts a handler-bind inside a handler-case:

(handler-case                     ; outer: makes the retry / give-up decision
    (handler-bind                 ; inner: runs FIRST, in the signalling context
        ((error (lambda (c) (log-the-live-backtrace c))))
      (funcall refresh handler))
  (error (c) (decide-retry c)))

Common Lisp runs the most recently bound handler first, so the inner handler-bind sees the condition while the stack is still live, logs the real signalling frame, and then declines: it returns normally rather than transferring control. Only then does the outer handler-case unwind and make the retry decision. Invert the nesting and the backtrace is gone before anything records it.

The handler logs the condition's type and a backtrace, not just its printed message. When a failure is rare or environment-dependent, that captured frame is the only instrument that will explain it: the tests for this path assert that the failure is logged with a backtrace, not merely survived.

.mallet.lisp names the files that carried masking handlers when the gate went up, each with its count. Those files are exempt so the gate could be turned on without a codebase-wide rewrite; every other file reports immediately on a new ignore-errors, in full, without refusing the commit.

The counts are there to be driven down. Retiring a file means converting its sites to the shapes above and deleting its line from the config, at which point the file is permanently gated. Do not add a file to that list. If you are touching a listed file anyway, retiring it is the cheapest it will ever be.

Raw counts do not give the triage order, and following them sends effort at the wrong files. The four largest (src/main.lisp, src/fabric.lisp, src/edge/seam.lisp, src/transport/keyed-acceptor.lisp) hold 79 of the 115 sites and look like the obvious place to start. They are in fact the least indebted files in the repo: 67 cleanup, 5 masking. Daemon lifecycle code is mostly symmetric teardown, and symmetric teardown is what cleanup looks like.

valis's actual masking debt concentrates on the ACME and certificate axis: src/active/cert-watch.lisp, src/active/acme-manager.lisp, src/active/capability.lisp. Triage by position first, then rank by what a masked failure would actually cost. Counting is not triage.

The highest-value finding in valis's own triage was not a large count. It was that *next-renewal-fn* has exactly two callers (the ACME renewal machinery, and the certificate-expiry watchdog that exists to catch a silent failure in that machinery) and both wrap it in ignore-errors. No other belt exists. If that seam starts signalling, the manager churns a renewal every boot while the watchdog's next-renewal reads NIL and its check never fires. The watcher shares the failure mode of the thing it watches, so the mechanism installed to detect the failure is removed by the same defect.

Look for this shape specifically: a guard, probe, monitor, or watchdog that masks the same call as the thing it guards. It is where masking costs the most, and a count-ordered triage will never surface it.

A third shape hides among the masking sites and is neither cleanup nor a handler problem: ignore-errors used as a validity test.

(ignore-errors (parse-integer token :junk-allowed nil))   ; NIL means "not a number"

The obvious-looking fix (reach for :junk-allowed t) is wrong, and wrong in two opposite directions depending on the call site:

• It accepts what should be rejected. (parse-integer "12abc" :junk-allowed t) returns 12. A malformed protocol parameter is silently taken as valid, where the masking at least failed closed.
• It rejects what is currently accepted. With :junk-allowed NIL, (parse-integer "2 ") returns 2 with an end index of 2 (the whole string), so a caller checking ( end (length text))= accepts it. With :junk-allowed t the end index is 1, that check fails, and the input is now refused.

:junk-allowed t is not looser or stricter. It is a different parse with a different whitespace contract, and which way it breaks depends on what the caller does with the second return value. A reviewer watching for the first hazard will wave the second one straight through.

The criterion:

• handler-case on parse-error, the default, and the only safe choice for untrusted or externally-authored input. It preserves the exact accept/reject contract including whitespace, and narrows the caught class to the one actually expected, which is what this document asks for anyway.
• :junk-allowed t only where both hold: trailing junk is genuinely acceptable, and nothing downstream depends on the end index or on surrounding-whitespace behaviour. In practice, a trusted config value or environment variable. The second condition is the one that gets missed: it is invisible unless you read what the caller does with the second value.
• Neither, where the whole token must be digits. valis's own two sites (src/main.lisp:1015 and :1028) require exactly that, already fail loud, and are correct as written.

The general point: a fix prescribed by shape is the same error as a triage by shape. Work out what the replacement would do at that call site before applying it.

A genuinely ignorable failure is suppressed at the site, with a reason:

; mallet:suppress no-ignore-errors -- <why this failure is genuinely ignorable>

The reason is the point. "Pre-existing" and "not important" are not reasons; a reader must be able to tell what failure is being accepted and why accepting it is safe. The linter's stale-suppression rule reports suppressions that are no longer needed, so these do not silently accumulate once the code beneath them improves.