This is a submission for DEV's Summer Bug Smash: Smash Stories powered by Sentry.
var fx = (rates.rates && rates.rates[v.currency]) || 1...
For further actions, you may consider blocking this person and/or reporting abuse
The post makes a useful distinction between a feature working once and a system being dependable. I’d add an explicit failure-mode checklist so the next contributor can see which assumptions are intentional and which ones still need evidence.
Fair — a failure-mode checklist would make the intent explicit rather than
implied. The three states here were "have it", "don't have it", "don't know
how old it is", and only the third was missing; writing them out beforehand
would have shown the gap without needing the bug.
Right — a missing value needs to preserve its uncertainty instead of acquiring a convenient default. Treating absence as a real state gives the operator a chance to investigate rather than letting a downstream calculation turn it into a plausible fiction.
That is the sentence I was reaching for and did not find. "Preserve the
uncertainty" is the actual requirement; a default is just the shape it loses.
The follow-on I got wrong the first time is that the instrument has the same
problem. My failure counter could not tell "nothing failed" from "the counter
itself has been throwing since the last deploy" — one absent state, one
convenient reading. It now records attempts alongside failures, so 0 of 8,640
and 0 of 0 stop printing the same line.
Exactly. The attempt count turns a zero from a claim into an observable:
0 failures / 8,640 attemptsand0 failures / 0 attemptscarry completely different operational meaning. I’d also emit an explicit instrumentation-health state, so a broken counter cannot silently masquerade as a clean service.Both directions of that bit me in one morning.
A cloaking check I added last week only spoke when it found cloaking, so a
clean day and a check that never ran printed the same thing: nothing. And a
file-integrity check alarmed every single morning on a log the cron writes
nightly — which trains the reader to skim the one line that matters.
Silent-when-healthy and loud-when-fine are the same defect wearing opposite
clothes. Both now report their own state explicitly, which is your point:
the instrument has to say it is alive, not just say when it is unhappy.
The
> 0guard moves the problem out of display and into the refusal branch, and that branch has the same property the substituting one had: it only runs when the upstream is partially down, a state you say you have never seen in a browser and never will on purpose. What I would add is a counter at the point of refusal rather than only an error string, so a partially-down feed leaves a trace you can read afterwards instead of a message one visitor saw once. It also gives the branch a heartbeat, because a counter that never moves across months in which your currency provider did have incidents is telling you the refusal path stopped being reachable.This is the right criticism and I'd only move where the counter goes. The
browser is the wrong place to keep it — each visitor's count dies with the
tab, and the one person who saw the refusal is the one person who can't tell
me.
The server already knows: LiveRates::fx() is where the provider fails and
falls back to ['USD' => 1.0], and that's the moment worth counting. Counting
at the display point measures how many people met a degraded state; counting
at the source measures how often it happened, which is the number I'd act on.
Your heartbeat point is the part I hadn't considered. A refusal counter that
stays at zero through a month when the provider did have incidents isn't good
news — it means the branch stopped being reachable and something upstream is
swallowing it.
Agreed on the source, with one thing worth nailing down at that point. The counter has to be incremented inside the fallback branch itself, not derived later by noticing the table came back as
['USD' => 1.0], because a healthy provider response can legitimately carry 1.0 for USD and the two are the same shape once they leavefx(). Anything reading the value downstream cannot tell them apart, so the count quietly turns into a count of something else. The two numbers are not substitutes either, since the source count says how often the provider failed and the display count says whether anyone was standing there when it did, and it is the gap between them that tells you the refusal path is still wired.Implemented, and your refinement is the reason it works rather than a detail on
top of it. The increment sits inside the branch that failed:
Deriving it downstream would have been wrong for exactly the reason you gave.
A healthy response legitimately carries 1.0 for USD, so once that array leaves
fx() a fallback and a good day are the same shape, and the count silently
becomes a count of something else.
On the two numbers not being substitutes — that changed what I store. It is one
integer per provider per month rather than a single running total, because "the
provider failed 40 times" and "it failed 40 times in one afternoon in March"
are different facts and only the second one is actionable.
Kept in a file rather than a table: it has to survive a deploy, and a counter
must never be the reason a price fails to render, so the whole thing sits
inside a try that swallows.
And it is printed in the daily report even when it reads zero. A counter nobody
looks at is the failure it was built to catch — which is your heartbeat point,
and the part I would not have got to on my own.
The
trythat swallows is the part I would instrument next, because it puts the counter in the same position the price was in. A zero in the daily report now reads either as no fallback happened or ascountFailurethrew on every call, and a file write is exactly the thing that starts failing quietly after a deploy changes the path or the owner. The cheapest fix that keeps the swallow is to increment a total on the success branch too, so the report prints failures against attempts.0 of 8640is a working instrument reporting good news and0 of 0is the instrument itself down, and right now those two print the same line.You are right, and it is the same defect the post was about. A lone zero could
not tell "nothing failed" from "this has been throwing since the last deploy",
which is a value that cannot distinguish two states — exactly what I wrote 900
words complaining about.
Both outcomes are tallied now, and the report reads failures against attempts:
fx: 0 fallbacks of 8,640 calls this month.
and if the denominator is zero it is raised as a problem rather than passed
over, with the reason spelled out — 0 of 0 means the counter is not being
reached, which is not the same as nothing failing.
Your point about the file write specifically is the part I would not have got
to. It is not a hypothetical: this deploys by unpacking a tar over an existing
tree, so a path or an owner changing under the cache directory is a completely
ordinary Tuesday, and it would have failed in the one way that leaves the
report looking healthy.
The
> 0rather than a truthiness check is the part I would underline, because one layer down the same three-valued logic goes the other way. A Postgres CHECK only rejects a row when its expression is false, and NULL passes.I hit that in a migration meant to stop a record going active without recorded proof of ownership. With the proof column NULL,
jsonb_typeof(NULL)is NULL, the AND chain is NULL, andfalse OR NULLis not a violation, so the absent case was the one thing that slipped through the gate written to catch it. One leadingIS NOT NULLfixed it.Which makes me wonder about yours: does the new
freshness()get a test that passes null, or only one that passes a number?Thanks — and yes, both. The first two assertions in the file are
freshness(null) and freshness(undefined), because "age unknown" was the whole
missing branch and a test that only passes a number would agree with the buggy
version too.
Your Postgres example is the same three-valued logic pointed the other way,
and it's worse than mine: my null produced a reassuring sentence, yours
produced a passing CHECK. A constraint that only rejects on
falseand letsNULLthrough is a gate that opens for exactly the row it was written tostop. The leading IS NOT NULL is the same fix as my
> 0— say what yourequire instead of trusting what's truthy.