DEV Community

Felix Yang
Felix Yang

Posted on

Your fetch failed, your code wrote an empty array, and now your tests assert a lie

A scraper of mine reported that a GitHub repository had zero closed pull requests.

The repository had dozens. It had its first PR closed 21 minutes after it was created, three days before the scraper ran.

Nothing crashed. No test failed. The number was produced, written to disk, committed, and then quoted in a report as evidence. The bug was not that the scraper lied — it is that my pipeline had no vocabulary for "I don't know", so it said 0 instead.

The chain of custody of a wrong number

Here is the whole failure, step by step.

1. A timeout that didn't look like a timeout.

The fetch was a curl call with a 25-second ceiling:

curl -sS --max-time 25 \
  "https://api.github.com/repos/$REPO/pulls?state=closed&per_page=100"
Enter fullscreen mode Exit fullscreen mode

A large repo returned ~606 KB. The body did not finish in 25 seconds, so curl killed the transfer mid-flight. curl still exited having written something to stdout, and — this is the part that matters — stdout contained the first N bytes of a perfectly valid JSON array, cut off in the middle of a string.

2. The parse error was swallowed.

try:
    data = json.loads(out)
except Exception:
    data = []          # <- the entire bug lives on this line
Enter fullscreen mode Exit fullscreen mode

One except Exception. It was written to be defensive and it is exactly what turned a truncation into a measurement.

3. The empty list was written to disk as a result.

Downstream code received [], counted it, and stored closed_prs: 0. A reader of that file cannot distinguish it from a repo that genuinely has no closed PRs. The snapshot format had one slot where there should have been two.

4. The empty list was promoted to a test fixture.

This is the expensive part. A regression fixture was created to cover the case "we found no evidence, so we must answer UNKNOWN". A real repository was picked as the realistic example, and the recorded value was closed_prs: 0 — read straight from the poisoned snapshot. From that commit onward, the test suite asserted a false statement about a third party, forever, and passing tests meant the bug was invisible.

5. The only reason it was caught was that the repo was small enough to enumerate every PR with its own timestamp. On a repo with 900 PRs there would have been no way to notice, and the false fact would still be in the fixture today.

Two variations of the same bug

Once I started looking, the same shape showed up elsewhere in the same codebase.

Variant A — a successful HTTP call that means "you are not authenticated".

curl -sS -H "Authorization: Bearer $TOKEN" "$API/runs"
# HTTP 401
# body: {"error": {"type": "unauthorized", "message": "Invalid token"}}
Enter fullscreen mode Exit fullscreen mode

The check was: did curl succeed, and does the body parse as JSON? Both true. Therefore runs = []. The dashboard reported zero runs for weeks. The token had expired on day one.

An auth failure had been silently converted into a zero — and zero is a plausible number for runs, which is why nobody questioned it.

Variant B — the timeout that leaves no trace at all.

--max-time bounds the whole transfer. It does not bound the connect phase. Measured hang before the failure surfaced:

curl -sS --max-time 30 "$URL"
# ... 268,008 ms later ...
# curl: (28) Operation timed out
Enter fullscreen mode Exit fullscreen mode

Four and a half minutes, with a 30-second timeout on the command line. If your retry logic lives above this call, your retry logic never ran. --connect-timeout is a separate, non-negotiable flag:

curl -sS --connect-timeout 8 --max-time 30 "$URL"
Enter fullscreen mode Exit fullscreen mode

And a hung first call was enough to abort the entire scan and discard three windows of already-collected data, because one unguarded call sat outside every try.

The rule: three states at every boundary

The fix is not "add more error handling". It is to make EMPTY and ERROR different types that cannot be compared or coerced into each other, so that the compiler — or at least the first reader — sees the difference.

I now return a small tri-state from every fetch, and nothing downstream sees a bare list:

from dataclasses import dataclass

OK, EMPTY, ERROR = "ok", "empty", "error"

@dataclass(frozen=True)
class Fetch:
    status: str          # OK | EMPTY | ERROR
    items: list
    detail: str = ""     # why it failed; never empty when status == ERROR

    def require(self):
        if self.status != OK:
            raise RuntimeError(f"fetch {self.status}: {self.detail}")
        return self.items
Enter fullscreen mode Exit fullscreen mode

Four properties are doing the work here.

1. ERROR can never be downgraded. A helper that returns [] on failure is not defensive, it is a lie generator. If a caller wants to continue, it must opt in explicitly, and the failure must survive into the output.

2. EMPTY must be earned. An empty list is only a fact when the transport completed and the parser ran and the shape was checked. Anything else is ERROR:

def fetch_json(url, timeout=(8, 30)):
    r = subprocess.run(
        ["curl", "-sS", "-w", "\n%{http_code}",
         "--connect-timeout", str(timeout[0]),
         "--max-time", str(timeout[1]), url],
        capture_output=True, text=True)

    if r.returncode != 0:
        return Fetch(ERROR, [], f"curl rc={r.returncode}: {r.stderr.strip()[:200]}")

    body, _, code = r.stdout.rpartition("\n")
    if code.strip() != "200":                       # read the status, never infer it
        return Fetch(ERROR, [], f"http {code.strip()}: {body.strip()[:200]}")

    try:
        data = json.loads(body)
    except json.JSONDecodeError as e:               # truncation lands here
        return Fetch(ERROR, [], f"json error: {e}")

    if not isinstance(data, list):
        return Fetch(ERROR, [], f"unexpected shape: {type(data).__name__}")

    return Fetch(OK if data else EMPTY, data)
Enter fullscreen mode Exit fullscreen mode

Note what is not in that function: no bare except, and no default value that is also a legitimate result.

3. Snapshot writers must refuse to write on failure. A partial or empty write is worse than no write, because the next run diffs against it and reports fabricated change:

def save_snapshot(path, windows):
    ok = [w for w in windows if w.status == OK]
    err = [w for w in windows if w.status == ERROR]

    if not ok:                       # nothing succeeded -> write nothing at all
        raise SystemExit(2, f"no window fetched: {[w.detail for w in err]}")

    if err:                          # partial -> write, but name what is missing
        print(f"WARNING: {len(err)} window(s) missing: "
              f"{[w.name for w in err]}")

    path.write_text(json.dumps([w.items for w in ok]))
Enter fullscreen mode Exit fullscreen mode

The rule underneath: the file on disk must never be indistinguishable from a successful run.

4. Errors must carry their own diagnosis. detail is why the failure is debuggable six weeks later without re-running anything. "fetch error: 401" is a fact; "fetch error" is a shrug.

Testing the failure path without lying to yourself

Two things went wrong in how this code was tested.

Assert on the value that can actually be wrong. The original test asserted the number of matches found by the parser. The broken code produced the same number of matches — it was wrong about which row and what value, not about how many. Old code and new code both passed. If a test would pass with the bug present, it is not testing the bug. Assert the field that moves:

assert [p["closed_at"] for p in result] == ["2026-10-15T20:21:49Z"]
Enter fullscreen mode Exit fullscreen mode

Use orthogonal single-point failures, not just total failure. "Everything fails" is the easy case and it exercises the least code. Run the pipeline twice: once with every fetch failing (expect: no file written, non-zero exit), and once with exactly one named window failing while the others succeed (expect: file written, missing window named, exit 0). The second run is the one that catches the swallowed exception.

Never let captured data masquerade as a synthetic fixture. A fixture harvested from a live third party is a claim about the world, and the world changes. If you keep one, store the command, the timestamp, and the exit status alongside it — or replace it with a hand-written fixture and record why. A fixture that asserts a durable fact about a mutable external system is a scheduled lie.

The generalisation

Every one of these bugs is the same bug: a value that means "I don't know" was given the same representation as a value that means "there is nothing".

It is worth hunting for on purpose, because the symptoms are quiet:

  • A function with except Exception: return [].
  • A pipeline that checks if response.ok but never response.status_code.
  • Any retry loop wrapping a call whose timeout doesn't cover connecting.
  • Any snapshot, cache, or fixture file whose contents are valid in both the "ran and found nothing" and "never ran" cases.
  • Any test that asserts a count.

None of these raise. They all produce numbers. And a pipeline that cannot say "I don't know" will say 0 instead, which is a much more expensive answer.

Top comments (0)