Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion .github/instructions/examples.instructions.md
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ against a CHR, showing both the **CLI and the library API**.
|---|---|---|
| `<name>.ts` | **Primary** — runnable Bun script, library API. `#!/usr/bin/env bun`; `if (import.meta.main) main()`; `try…finally` teardown; `process.exitCode = 1` on failure. | always (except `grounding`) |
| `<name>.sh` | CLI version — POSIX `sh`, sources `../common.sh`. | most |
| `<name>.ps1` | PowerShell CLI mirror, dot-sources `../common.ps1`. | **all new** examples; existing where the CLI flow is simple |
| `<name>.ps1` | PowerShell CLI mirror. Sets `$ErrorActionPreference = 'Stop'` **before** dot-sourcing `../common.ps1` (see "Exit 0 is not evidence"). | **all new** examples; existing where the CLI flow is simple |
| `<name>.py` | Python CLI driver, run with `uv run` (stdlib only). | where a non-TS audience adds value |
| `<name>.test.ts` | `bun:test` — only when assertions ARE the documentation. | `grounding` only |
| `README.md` | from `_template/README.md`. | always |
Expand Down Expand Up @@ -50,6 +50,37 @@ in [`COVERAGE.md`](../../examples/COVERAGE.md) (mark docs/test-only with a reaso
run in extended verification, across the supported-OS matrix. `trial-license`
is manual-only (rate limits).

## Exit 0 is not evidence ([#102](https://github.com/tikoci/quickchr/issues/102))

An example that exits 0 and prints *something* is not a passing example. Two
instances shipped green for months:

- A `ParserError` in `common.ps1` took out the whole file, so every helper in
`quickstart.ps1` was undefined. No quickchr command ran; the script printed one
line and exited 0.
- `mndp.py`'s "no announcement received" branch used a bare `return`, which leaves
`main()` through the `finally` and never reaches its `sys.exit(rc)` — so the
failure path exited 0.

Three rules, each holding one end of that:

- **The smoke harness asserts output markers**, not just `code === 0` and
`out.length > 0`. Each entry in `examples-smoke.test.ts`'s `RUNNABLE` names
substrings only a working run produces — including one from the END of the script
and one the CLI/library emitted rather than the example's own `echo`.
- **`.ps1` sets `$ErrorActionPreference = 'Stop'` before dot-sourcing `common.ps1`**,
duplicating what `common.ps1` sets. Load-bearing, not stylistic: if `common.ps1`
fails to load, its own preference never takes effect and each undefined helper is
a non-terminating `CommandNotFound`. Reproduced 2026-08-03 (pwsh 7.4.6, Intel
macOS) against the real files with #102's defect reinstated: rc=0 without the
guard, rc=1 with it. Enforced by `scripts/validate-examples.ts`.
- **A failure path must exit non-zero.** In Python `raise SystemExit(msg)`, never a
bare `return` past a trailing `sys.exit(rc)`. Same run: rc=0 before, rc=1 after.

`lint-powershell.yml` parses every `.ps1` with PowerShell's own parser before
PSScriptAnalyzer, because a `Severity = @('Error','Warning')` filter does not
surface `ParserError` — a file that cannot be parsed at all passed that gate.

## A failing example is a quickchr bug until proven otherwise

Examples are **canaries**, not chores. The reason each one boots a real CHR is to catch
Expand Down
39 changes: 39 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,45 @@ Even minor versions (0.2.x, 0.4.x) are releases; odd minors (0.3.x, 0.5.x) are p

### Fixed

- **One refused connection to `upgrade.mikrotik.com` failed a whole `quickchr start`**
(#121). `resolveVersion()` had no retry at all: `fetchResilient()` is one attempt per
transport (system resolver, then a public-DNS IPv4 fallback), and if the fallback also
threw, the error propagated. #116/#119 gave the *download* path a retry policy and left
version resolution — which sits on the critical path of `start()`, the wizard and the
CLI — with a single shot. Observed on a `platforms=all` run: `Integration
(windows/x86_64 · testing)` went red on one `ConnectionRefused`, on a leg where nothing
about RouterOS, QEMU or the platform was involved. `resolveVersion()` now retries a
connection-class failure up to 3 times with the same backoff shape as `downloadToFile`,
and reports the attempt count plus the host when it exhausts them. An HTTP status is
still terminal on the first attempt — a 404 for a bad channel stays fast.
- **The version-resolution error named an IP address nobody configured** (#121).
`fetchResilient()` discarded the direct attempt's error whenever the IPv4 fallback
itself threw, so the surfaced message read `path: "https://159.148.147.251/routeros/…"`
with no hostname — leaving "the system resolver was broken", "the host refused us" and
"public DNS handed back a bad address" indistinguishable, which is exactly the question
the log has to answer. That fallback fires routinely on hosted runners (their stub
resolver fails slowly, 2–26 s), so its error was often the only one visible. Both
failures are now raised together as a `ResilientFetchError` naming the URL, each
transport's error, the address the fallback used and which resolvers produced it, with
the direct failure kept as `cause`.
- **Every PowerShell example was a silent false green** (#102). A `ParserError` in
`examples/common.ps1` took out the entire file — PowerShell parses `$LASTEXITCODE:` as
a scope-qualified variable reference — so every helper it defines was undefined for the
caller and `quickstart.ps1` ran no quickchr command at all. It exited 0 having printed
one line, and the smoke harness (`code === 0` and `out.length > 0`) reported `(pass)`
for months. The parse itself was fixed in #101; the false-green *class* is closed here:
the smoke harness now asserts per-example output markers — substrings only a working
run produces, including one from the end of the script and one the CLI itself emitted —
and each `.ps1` sets `$ErrorActionPreference = 'Stop'` **before** dot-sourcing
`common.ps1`, so a `common.ps1` that fails to load can no longer leave the example
running past it. Reproduced locally against the real files (pwsh 7.4.6, Intel macOS):
with #102's defect reinstated, rc=0 unguarded and rc=1 guarded. `validate-examples`
enforces the guard.
- **`examples/mndp/mndp.py` exited 0 when no MNDP announcement arrived.** The failure
branch used a bare `return`, which leaves `main()` through the `finally` and never
reaches the trailing `sys.exit(rc)` — the same false green as #102, in a different
language. It now raises `SystemExit`; cleanup still runs. Measured with the identity
match forced to fail: rc=0 before, rc=1 after.
- **A healthy large download was reported as a timeout** (#116). Both download paths
bounded a transfer by *total duration*: `images.ts` aborted at a flat 120 s per
attempt and retried three times, `packages.ts` had no deadline and no retries at all.
Expand Down
7 changes: 7 additions & 0 deletions examples/common.ps1
Original file line number Diff line number Diff line change
@@ -1,10 +1,17 @@
# Shared helpers for quickchr CLI examples (PowerShell - the Windows mirror of common.sh).
#
# Dot-source at the top of <name>.ps1, then wrap the body in try/finally:
# $ErrorActionPreference = 'Stop'
# . "$PSScriptRoot/../common.ps1"
# $name = Get-ExampleName 'quickstart'; Register-Cleanup $name
# try { Invoke-Qc start $name --channel stable; ... } finally { Invoke-QcCleanup }
#
# The caller sets $ErrorActionPreference BEFORE the dot-source even though this
# file sets it too, and that duplication is load-bearing: if THIS file fails to
# load, the preference it sets never takes effect, every helper call below it is
# an ordinary non-terminating CommandNotFound, and the example exits 0 having done
# nothing - the silent false green in #102. Enforced by scripts/validate-examples.ts.
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
#
# Resolution rule mirrors common.sh: prefer an explicit $env:QUICKCHR override,
# else the repo source CLI (so CI/local runs exercise THIS checkout), else a
# globally installed `quickchr`.
Expand Down
1 change: 1 addition & 0 deletions examples/device-mode/device-mode.ps1
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# device-mode (CLI, Windows) - enable a device-mode feature on first boot, read it back.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'device-mode'
Expand Down
1 change: 1 addition & 0 deletions examples/dude/dude.ps1
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# dude (CLI, Windows) - install the dude package on first boot, enable it, read it back.
# PowerShell mirror of dude.sh.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'dude'
Expand Down
1 change: 1 addition & 0 deletions examples/grounding/grounding.ps1
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# grounding (CLI, Windows) - apply RouterOS config, read it back, prove it took.
# PowerShell mirror of grounding.sh.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'grounding'
Expand Down
1 change: 1 addition & 0 deletions examples/harness/harness.ps1
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# harness (CLI, Windows) - hand a CHR's connection env to an external tool.
# PowerShell can't eval the shell-quoted `env` output, so use --json and set
# $env:* explicitly. This is the natural Windows showcase for env-passing.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'harness'
Expand Down
20 changes: 17 additions & 3 deletions examples/mndp/mndp.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,19 @@


def run_quickchr(*args: str, check: bool = True) -> subprocess.CompletedProcess:
return subprocess.run([*QUICKCHR, *args], capture_output=True, text=True, check=check)
proc = subprocess.run([*QUICKCHR, *args], capture_output=True, text=True)
if check and proc.returncode != 0:
# subprocess.run(check=True) raises a CalledProcessError naming the command
# and the exit code and DISCARDING both streams -- and those streams are the
# only place quickchr says what actually went wrong. Run 30852139131
# (linux/aarch64) failed here on `/system/identity/set` and reported nothing
# but "returned non-zero exit status 1", which is not enough to diagnose it.
raise SystemExit(
f"FAIL: quickchr {' '.join(args)} exited {proc.returncode}\n"
f"--- stdout ---\n{proc.stdout}"
f"--- stderr ---\n{proc.stderr}"
)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
return proc


# ── frame parsing ────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -234,8 +246,10 @@ def main() -> None:
time.sleep(1)

if not got:
print(f"FAIL: no MNDP announcement with identity '{IDENTITY}' within {args.timeout}s", file=sys.stderr)
return
# `raise`, not `return`: a bare return leaves main() through the finally
# and never reaches `sys.exit(rc)` below, so the failure would exit 0 —
# the same silent false green as #102's PowerShell example.
raise SystemExit(f"FAIL: no MNDP announcement with identity '{IDENTITY}' within {args.timeout}s")
print("\nMNDP received over L2 (socket-connect):")
for k in ("identity", "version", "platform", "board", "ifname", "ipv4", "uptime", "mac", "softwareId"):
if k in got:
Expand Down
1 change: 1 addition & 0 deletions examples/quickstart/quickstart.ps1
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# quickstart (CLI, Windows) - boot a CHR, read resource + descriptor, tear down.
# PowerShell mirror of quickstart.sh. try/finally guarantees teardown.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'quickstart'
Expand Down
1 change: 1 addition & 0 deletions examples/rollback/rollback.ps1
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# rollback (CLI, Windows) - snapshot a CHR, change it, restore the snapshot.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'rollback'
Expand Down
1 change: 1 addition & 0 deletions examples/service-forward/service-forward.ps1
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
# service-forward (CLI, Windows) - pin a guest service to a chosen host port.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'service-forward'
Expand Down
1 change: 1 addition & 0 deletions examples/trial-license/trial-license.ps1
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
# trial-license (CLI, Windows) - apply a CHR trial license, read it back. MANUAL-ONLY.
# MikroTik rate-limits trial requests, so this is excluded from CI.
$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"

$name = Get-ExampleName 'trial-license'
Expand Down
1 change: 1 addition & 0 deletions examples/version-matrix/version-matrix.ps1
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
# PowerShell mirror of version-matrix.sh (parallel start via background jobs).
param([switch]$Lite)

$ErrorActionPreference = 'Stop' # before the dot-source - see common.ps1 (#102)
. "$PSScriptRoot/../common.ps1"
Comment thread
mobileskyfi marked this conversation as resolved.

$channels = if ($Lite) { @('long-term', 'stable') } else { @('long-term', 'stable', 'testing', 'development') }
Expand Down
21 changes: 20 additions & 1 deletion scripts/validate-examples.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,11 @@
* - no Makefile / stray files (only .ts/.test.ts/.sh/.ps1/.py/.md + known subdirs)
* - the primary script is named after its directory
* - every relative link and run-command file reference in the README resolves
* - each .ps1 sets $ErrorActionPreference before dot-sourcing common.ps1
*
* Wired into `bun run check`. Exits non-zero on any violation.
*/
import { readdirSync, statSync } from "node:fs";
import { readFileSync, readdirSync, statSync } from "node:fs";
Comment thread
coderabbitai[bot] marked this conversation as resolved.
import { existsSync } from "node:fs";
import { join, resolve } from "node:path";

Expand Down Expand Up @@ -80,6 +81,24 @@ for (const name of dirs) {
}
}

// PowerShell examples must arm $ErrorActionPreference themselves, BEFORE
// dot-sourcing common.ps1. A common.ps1 that fails to load leaves the
// preference at 'Continue', so every undefined helper is a non-terminating
// error and the example exits 0 having run no quickchr command at all — the
// false green in #102, reproduced locally 2026-08-03 (pwsh 7.4.6: rc=0 without
// the guard, rc=1 with it).
for (const f of files.filter((x) => x.endsWith(".ps1"))) {
const lines = readFileSync(join(dir, f), "utf8").split(/\r?\n/);
const dotSource = lines.findIndex((l) => /^\s*\.\s+["']\$PSScriptRoot/.test(l));
if (dotSource < 0) continue; // self-contained script, nothing to guard against
const armed = lines
.slice(0, dotSource)
.some((l) => /^\s*\$ErrorActionPreference\s*=\s*'Stop'/.test(l));
if (!armed) {
err(where, `"${f}" must set $ErrorActionPreference = 'Stop' BEFORE dot-sourcing common.ps1`);
}
}

// README references resolve.
const readmePath = join(dir, "README.md");
if (existsSync(readmePath)) {
Expand Down
62 changes: 59 additions & 3 deletions src/lib/net.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,9 @@
* error), we retry by resolving the A record against a public DNS server
* directly (~10 ms), bypassing the host's resolv.conf, and connecting to the
* IPv4 literal with the `Host` header and TLS SNI preserved so certificate
* validation still passes.
* validation still passes. If that attempt fails too, both failures are reported
* together as a `ResilientFetchError` — the fallback's own error names only an IP
* literal, which on its own reads as a host nobody configured.
*
* Only connection-class failures trigger the fallback; HTTP responses (incl.
* 5xx) and aborts (`AbortError`, e.g. from `AbortSignal.timeout`) pass through
Expand All @@ -32,16 +34,64 @@ import { promises as dns } from "node:dns";
const PUBLIC_DNS_SERVERS = ["1.1.1.1", "8.8.8.8", "1.0.0.1"];
const DNS_TIMEOUT_MS = 3000;

/** `CODE: message` when the error carries a code, else just the message. */
function describeError(err: unknown): string {
if (!err || typeof err !== "object") return String(err);
const e = err as { name?: string; message?: string; code?: string; cause?: { code?: string } };
const code = e.code ?? e.cause?.code;
const label = code ?? e.name;
const message = e.message ?? String(err);
return label ? `${label}: ${message}` : message;
}

/**
* Both transports failed: the direct fetch with a connection-class error, and
* then the public-DNS/IPv4 fallback too.
*
* It exists because the fallback's own error is unreadable on its own — it names
* an IP literal nobody configured and no hostname, so a log cannot distinguish
* "the system resolver was broken" from "the host refused us" from "public DNS
* handed back a bad address" (#121). The message carries the URL, both failures,
* and which address the fallback tried; the direct failure is also the `cause`.
*/
export class ResilientFetchError extends Error {
/** The hostname both transports were aiming at. */
readonly host: string;
/** The IPv4 literal the fallback connected to, from public DNS. */
readonly address: string;
/** The fallback transport's failure. The direct attempt's is `cause`. */
readonly fallbackError: unknown;

constructor(url: string, address: string, directError: unknown, fallbackError: unknown) {
const host = new URL(url).hostname;
super(
`Fetch failed for ${url} on both transports: ` +
`system resolver — ${describeError(directError)}; ` +
`public DNS (${PUBLIC_DNS_SERVERS.join(", ")}) → ${address} — ${describeError(fallbackError)}`,
{ cause: directError },
);
this.name = "ResilientFetchError";
this.host = host;
this.address = address;
this.fallbackError = fallbackError;
}
}

/**
* True for the connection-class failures raised when a socket cannot be opened
* (e.g. IPv4 blocked on an IPv6-only network, or Bun's `errno: 0`
* ConnectionRefused / FailedToOpenSocket against an unreachable address), plus
* the standard connect errors. Excludes `AbortError` (aborts/`AbortSignal`
* timeouts) and HTTP-level outcomes (those carry a Response and never throw
* here). Used to decide whether to fall back from the normal fetch to the
* public-DNS IPv4 attempt.
* public-DNS IPv4 attempt, and by callers deciding whether a failure is worth
* another attempt.
*/
export function isConnectionFailure(err: unknown): boolean {
// Connection-class by construction: fetchResilient raises this only after a
// direct failure that already passed this test. Callers retrying on a
// connection failure must keep retrying once the fallback also fails.
if (err instanceof ResilientFetchError) return true;
if (!err || typeof err !== "object") return false;
const e = err as { name?: string; code?: string; errno?: number; cause?: { code?: string } };
if (e.name === "AbortError") return false;
Expand Down Expand Up @@ -142,6 +192,12 @@ export async function fetchResilient(url: string, init?: BunFetchRequestInit): P
const address = await resolveIpv4(new URL(url).hostname);
// Public DNS can't help (unreachable / no answer) — surface the original failure.
if (address === undefined) throw err;
return fetchOverIpv4(url, address, init);
try {
return await fetchOverIpv4(url, address, init);
} catch (fallbackErr) {
// Never let the fallback's error stand alone: it names only an IP literal,
// which is the one thing the reader cannot map back to what was attempted.
throw new ResilientFetchError(url, address, err, fallbackErr);
}
}
}
Loading
Loading