Skip to content

Run Litestream commands without a shell and surface failures - #76

Open
cole-robertson wants to merge 1 commit into
fractaledmind:mainfrom
Rebulk:command-runner
Open

Run Litestream commands without a shell and surface failures#76
cole-robertson wants to merge 1 commit into
fractaledmind:mainfrom
Rebulk:command-runner

Conversation

@cole-robertson

Copy link
Copy Markdown

First of two to bring the gem to Litestream 0.5. This one stands alone and fixes a bug that exists on 0.3.13 today.

Problem

Commands.run joined argv into one shell string, ignored the exit status, and discarded stderr. A failing command returned "" (or [] after table parsing) and looked like success; arguments with spaces broke; argv reached the shell unescaped.

Reproduced against 0.5.17: ltx on a database that is not in the config exits 1 with Error: database not found in config on stderr and nothing on stdout. The old wrapper returned [], indistinguishable from "nothing replicated yet". The 0.3.13 binary uses the same exit and stderr conventions.

Change

One process path: Open3.popen3(*cmd, pgroup: true), argv array, no shell.

  • Non-zero exit raises CommandFailedException with the command, exit status, and stderr.
  • Explicit output mode instead of guessing from argv: :table (existing parsing), :raw, or :json when a caller passes json: true (Litestream ≥ 0.5). Restore's two opt-in skips (-if-db-not-exists on an existing output, -if-replica-exists with no backups) exit 0 and print one logfmt line on stdout even with -json; those return {"skipped" => true, "message" => ...}.
  • timeout: kills the process group (TERM, then KILL after a one-second grace, unconditionally) on expiry and raises CommandTimeoutException with the child reaped. The deadline also covers the pipe readers, so a descendant that outlives the direct child is caught.
  • Option values are stringified before spawning; Process.spawn rejects Integers, which the old shell-string runner accepted and the README documents (--parallelism 10).
  • The LITESTREAM_INSTALL_DIR note prints once per process.
  • The old error-row sniffing in execute is gone; it only existed because failures were swallowed.

Public method signatures and default return shapes are unchanged. prepare still returns argv.

Verified

  • 102 tests, 0 failures; standardrb clean; fork CI (Ruby workflow) green.
  • New TestRunner class drives a fake executable: table, JSON object/array, empty list, both skip lines, non-zero exit with stderr in the message, an argument containing a space, timeout with reap, timeout with a descendant that ignores TERM, an Integer option value, note printed once.
  • Real 0.5.17: databases in table and JSON modes, restore -json returning txid, the skip case, a failing restore raising with the stderr message, a 0.2 s timeout killing a sleeping fake with no orphan left.
  • Running in production at Rebulk (Nightrail Cloud) for the readiness probe and restore drill.

Commands.run joined argv into one shell string, ignored the exit status
and discarded stderr, so a failing command returned "" (or [] after
table parsing) and looked like success. Arguments with spaces broke, and
argv reached the shell unescaped.

The runner now uses Open3.popen3 with an argv array, raises
CommandFailedException with the exit status and stderr on failure, and
takes an explicit output mode: :table (the existing header/rows
parsing), :raw, or :json when a caller passes json: true (Litestream
>= 0.5). In JSON mode the two opt-in restore skips, which print one
logfmt line on stdout with exit 0, come back as {"skipped" => true,
"message" => ...} so callers can tell "did nothing" from data.

timeout: runs the command in its own process group and TERMs then KILLs
it on expiry, raising CommandTimeoutException with the child reaped.
The LITESTREAM_INSTALL_DIR note prints once per process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant