Run Litestream commands without a shell and surface failures - #76
Open
cole-robertson wants to merge 1 commit into
Open
Run Litestream commands without a shell and surface failures#76cole-robertson wants to merge 1 commit into
cole-robertson wants to merge 1 commit into
Conversation
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.
This was referenced Sep 10, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.runjoined 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:
ltxon a database that is not in the config exits 1 withError: database not found in configon 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.CommandFailedExceptionwith the command, exit status, and stderr.:table(existing parsing),:raw, or:jsonwhen a caller passesjson: true(Litestream ≥ 0.5). Restore's two opt-in skips (-if-db-not-existson an existing output,-if-replica-existswith 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 raisesCommandTimeoutExceptionwith the child reaped. The deadline also covers the pipe readers, so a descendant that outlives the direct child is caught.Process.spawnrejects Integers, which the old shell-string runner accepted and the README documents (--parallelism 10).LITESTREAM_INSTALL_DIRnote prints once per process.executeis gone; it only existed because failures were swallowed.Public method signatures and default return shapes are unchanged.
preparestill returns argv.Verified
TestRunnerclass 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.databasesin table and JSON modes,restore -jsonreturning 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.