balancer-lite-lua/.learnings/ERRORS.md

balancer-lite-lua — Lessons Learned

Three recurring footguns caught during the routing/wg/ctl wiring (commit daf240f). All three were silent: code parsed, tests passed, but end-to-end boot was broken. Per-module unit tests alone don't catch any of them — they only surface when you try to actually use the module from another module or boot the daemon.

---

1. gsub patterns treat \0 as zero-width match (Lua 5.1)

Symptom: JSON output looked like {"kind":"\u0000s\u0000w\u0000i\u0000t\u0000c\u0000h\u0000",...} — the \u0000 text was injected between every character of every string field.

Cause: In Lua 5.1, the pattern character \0 matches a zero-width position between every character (it's the "match anywhere" anchor, like \b in some regex flavors). So s:gsub('\0', 'X') on "abc" returns "XaXbXcX", not "abc".

Same trap applies to: \0 as a character class, and possibly other \d-style shortcuts. The null byte (0x00) is treated as the special "any position" anchor.

Fix: Don't use \0 in patterns. To match a literal null byte, use string.find(s, '\0', i, true) with the plain=true flag (which disables pattern matching), then rebuild the string manually. Example from src/balancerlite/json.lua:

-- BAD: silent zero-width match
v:gsub('\0', '\\u0000')

-- GOOD: explicit find + manual rebuild
local out, i = {}, 1
while true do
  local p = string.find(v, '\0', i, true)
  if not p then out[#out+1] = v:sub(i); break end
  out[#out+1] = v:sub(i, p-1) .. '\\u0000'
  i = p + 1
end
table.concat(out)

Reproduction:

lua5.1 -e "print(('abc'):gsub('\0', 'X'))"
-- Output: XaXbXcX  4   (NOT 'abc')

---

2. s and get_opt(...) or default silently coerces false → default

Symptom: UCI option option icmp_enabled '0' was loaded back as true (the default), so the smoke test's "all probes disabled" config actually ran all probes.

Cause: Lua's and/or short-circuit semantics: A and B or C evaluates to B when A is truthy, else C. When B itself is false (a valid bool opt value), the expression returns C instead — so an explicit false becomes the default.

Fix: Never use X and Y or Z when Y can be falsy. Use an explicit helper:

-- BAD: silently breaks for false / 0 / ""
icmp_enabled = sh and get_opt(sh, "icmp_enabled", true, "bool") or true
-- When sh is truthy AND get_opt returns false, expression returns the right-side true

-- GOOD: explicit branch
local function opt(s, k, default, conv)
  if not s then return default end
  return get_opt(s, k, default, conv)
end
icmp_enabled = opt(sh, "icmp_enabled", true, "bool")

Rule of thumb: A and B or C is safe only when B is guaranteed truthy (string, table, non-zero number, or true). For all other types use an explicit if.

Reproduction:

lua5.1 -e "local sh = {_options={enabled='0'}}
local get_opt = function() return false end
print(sh and get_opt() or true)"
-- Output: true  (the default, NOT the configured false)

---

3. Module new() defined as local but accessed via the module table

Symptom: lua5.1 src/balancerlite/main.lua --dry-run crashed with attempt to call field 'new' (a nil value) at the first mod.new(...) call.

Cause: Two of the existing modules had local function new(cfg) instead of function mod.new(cfg). The local function existed but was invisible to the return table — only mod.foo style functions were exposed.

-- src/balancerlite/probes.lua  (BEFORE)
local function new(cfg) ... end
function probes.all(...) ... end
return probes          -- probes.new is nil!

-- AFTER
local function new(cfg) ... end
function probes.all(...) ... end
probes.new = new
probes.all = probes.all  -- self-redundant but makes exports explicit
return probes

Fix: Either (a) define new as function mod.new(cfg) like the other exported functions, or (b) explicitly assign all exports before return mod. Option (a) is shorter; option (b) gives you a single "public API" section at the bottom.

Rule: When defining a module, every function intended to be called from outside must be reachable via the return table. local function foo is invisible to callers unless you re-bind it: mod.foo = foo or function mod.foo(...).

---

General lesson

End-to-end boot/parse/run is the only test that catches cross-module wiring bugs. Per-module unit tests catch logic bugs inside the module; lint catches syntax; smoke tests catch everything else. Always do an end-to-end run before committing.

The wiring work that exposed all three bugs here was: "run the actual daemon in dry-run mode after adding the new modules." That single command — lua5.1 main.lua --dry-run — found three silent failures in ~10 seconds. Cheaper than any amount of code review.

---

2026-09-04 — docker cp into a missing container path creates a nested dir

Symptom: make test green on testbed, but the container was running a stale copy of the fixture: ls /testbed/tests showed both the files and a nested tests/ subdir inside it. The runner executed /testbed/tests/... which resolved to the stale top-level files from a previous run, while my updated files sat in /testbed/tests/tests/.

Root cause: docker cp <src> container:/testbed/tests — when /testbed/tests doesn't exist, docker copies the source dir itself into the destination name, i.e. .../tests becomes /testbed/tests/ only if the target exists; otherwise you get the classic "dir copied as child" ambiguity (same as tar's behavior with missing targets). First copy of a run created the right layout; a later rm -rf /testbed/tests

• re-copy produced /testbed/tests as a file-like copy of the source directory, and a third re-copy then nested tests inside it.

Fix / rule: Before docker cp of a directory into a container, docker exec <c> sh -c 'mkdir -p <dest>' first (so the target exists and the copy lands inside it), or rm -rf then mkdir -p in the same sh -c. Verify with md5sum after the copy — a 2-second check that caught the stale-fixture bug that made a "PASS" run validate the wrong code.

Corollary: Testbed runners that re-derive filenames with patterns (f:match("tests/(%w+)%.lua")) silently break on names with underscores/hyphens (probes_config.lua → nil → attempt to concatenate a nil value). Match ([%w_]+) and assert the match is non-nil before building the output path.

Also (same day): the shellspec/openwrt 19.07 image ships no openssl — sha256.lua (openssl-CLI based) returned nil silently and its unit test's os.execute exit code still read 0, so the runner said "PASS" while 3 vectors actually failed. Two lessons: (1) record image package state in the testbed notes (opkg list-installed); install openssl-util once: opkg install openssl-util. (2) A test suite whose pass/fail is decided by printed text AND exit code must make the runner assert on the printed summary line, not just the exit code — FAIL: in output with exit=0 is a runner bug, not a test pass.