fix: docker-app check, error messages, and docs that did not work as written - #5
Merged
Merged
Conversation
added 3 commits
September 17, 2026 19:02
Two stacked bugs meant docker-app failed for every user, on every image, on every platform, in v0.1.0 and v0.1.1. docs/recipes-docker-app.md leads with it as the strongest verification available, the README lists it, and restorable init writes it (commented) into every generated recipe, so anyone uncommenting what the tool produced got exit 2. nat.NewPort takes the protocol first, then the port. The call passed them the other way round, so "tcp" was parsed as a port number and the check died before the container was created. git log -S shows the line unchanged since the original docker sandbox commit: it never worked. With that fixed, MappedPort surfaced a second bug. It inspected once, straight after start, but the daemon fills NetworkSettings.Ports asynchronously, so the binding is reliably absent for the first few hundred milliseconds. It now polls to a deadline. It also reports a container that exited before publishing as exactly that, rather than "port is not published", which sent users looking at networking when the real problem was an app dying on the restored data. Why this shipped: dockerapp_test.go asserts against a fake ContainerRunner, so it verifies the spec the check builds and never executes the code that consumes it, and no e2e covered docker-app at all. run-docker.sh now boots a real nginx against restored data and asserts both a pass and a readiness failure, and asserts the failure is never "not published" so the port race cannot come back silently. Confirmed the new cases are not vacuous: with the pre-fix docker.go restored, case 3 fails and the suite never reaches case 4.
Four message problems, each hit on a path a new user is likely to take.
The human result went to stderr, so `restorable test > result.txt` captured
nothing while --json captured everything. docs/commands.md promises "progress
goes to stderr, the result to stdout"; now both modes agree.
Database failures discarded the reason. psql prints the cause first and then
echoes the statement with a caret under it, so keeping the last two lines kept
the caret and dropped `relation "assets" does not exist`. Messages now start
from the line naming the failure:
before: count rows in "assets": LINE 1: SELECT count(*) FROM "assets" / ^
after: count rows in "assets": ERROR: relation "assets" does not exist /
LINE 1: SELECT count(*) FROM "assets"
The redaction added earlier still strips the LINE segment before transport, so
the reason travels and the row data does not.
A wrong password or mistyped repo path is the most common first-run mistake,
and it printed restic's raw JSON with the one useful sentence buried inside.
It now reads: restic snapshots: exit status 12: Fatal: wrong password or no
key found.
A missing agent.yaml said only that the file does not exist, which is unhelpful
to someone who has not met `restorable init` yet. It now names the command.
Every command in the quickstart is now runnable verbatim. Three were not. The cron line used RESTIC_PASSWORD_FILE, which the agent has never supported, so step 4 failed every time it ran. Worse, it failed into a log file nobody reads, which is precisely the silent-watcher failure this product exists to announce. It now passes the password the supported way. It also splits the streams rather than using 2>&1: with --json and stderr merged in, the results file was never parseable JSON. `restic ls latest` was offered as the way to discover paths to assert, at the exact moment a user is least sure what to write, but nothing in the quickstart ever sets RESTIC_REPOSITORY, so it errored out. Now passes -r. Step 5 said to register against https://<dashboard> and to "create an account on the dashboard" without saying where that is. The URL appears nowhere in the repo, so the step could not be completed. It names restorable.dev. recipes/immich.yaml could not pass for anyone. Immich names its dumps immich-db-backup-20250729T114018-v1.136.0-pg14.17.sql.gz, so the literal "immich-db-backup-latest.sql.gz" can never exist, and the asserted tables were renamed to singular upstream (verified: the schema declares @table('user') and @table('asset')). Since recipes take literal paths with no globbing, the postgres check is commented out with the shell line that gives the dump a stable name. A first run that fails for reasons that are not the user's backup is worse than no recipe. The README and agent-configuration both listed six read-only subcommands; the closed type has seven. SECURITY.md already said seven, so the repo contradicted itself on the one claim a security-minded reader will diff against the source. alerts.md attributed the hourly cron to vercel.json, which actually schedules the daily backstop. The hourly run is a GitHub Actions workflow, which is where you look when alerts are late, and which gets disabled after repository inactivity. Billing docs advertised enforced limits and a price while the live site gives everything away during the open beta. Repo readers arrive first, so they were seeing the worse offer.
dabelle
pushed a commit
that referenced
this pull request
Sep 19, 2026
fix: docker-app check, error messages, and docs that did not work as written
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.
The rest of the pre-launch sweep: the
docker-appcheck, the error messages a new user actually hits, and every documented command that did not work as written.docker-app has never worked
Two stacked bugs, in v0.1.0 and v0.1.1 both.
nat.NewPorttakes the protocol first, then the port. The call passed them the other way round, so"tcp"was parsed as a port number and the check died before the container was created.git log -Sshows the line unchanged since the original docker sandbox commit.With that fixed, a second bug surfaced:
MappedPortinspected once immediately after start, but the daemon fillsNetworkSettings.Portsasynchronously, so the binding is reliably absent for the first few hundred milliseconds. It now polls to a deadline, and reports a container that exited before publishing as exactly that, instead of "port is not published" sending people to look at networking when the real problem was an app dying on the restored data.This shipped because
dockerapp_test.goasserts against a fakeContainerRunner, verifying the spec the check builds while never executing the code that consumes it, and no e2e covereddocker-appat all.run-docker.shnow boots a real nginx against restored data and asserts both a pass and a readiness failure, and asserts the failure is never "not published" so the race cannot come back silently. Confirmed non-vacuous: with the pre-fixdocker.gorestored, case 3 fails and the suite never reaches case 4.Error messages
The human result went to stderr, so
restorable test > result.txtcaptured nothing while--jsoncaptured everything.docs/commands.mdpromises the opposite.Database failures discarded the reason. psql prints the cause first then echoes the statement with a caret under it, so keeping the last two lines kept the caret and dropped the explanation:
The redaction added in #4 still strips the
LINEsegment before transport, so the reason travels and the row data does not.A wrong password is the most common first-run mistake and it printed restic's raw JSON with the one useful sentence buried inside. It now reads
Fatal: wrong password or no key found. A missingagent.yamlnow namesrestorable initinstead of only reporting that a file is absent.Documentation that did not work
Every command in the quickstart is now runnable verbatim. Three were not.
The cron line used
RESTIC_PASSWORD_FILE, which the agent has never supported, so step 4 failed every time, into a log file nobody reads. That is the silent-watcher failure this product exists to announce. It also used2>&1alongside--json, so the results file was never parseable; the streams are now separated.restic ls latestwas offered as the way to discover paths to assert, at the moment a user is least sure what to write, but nothing in the quickstart setsRESTIC_REPOSITORY. Step 5 said to register againsthttps://<dashboard>without the URL appearing anywhere in the repo.recipes/immich.yamlcould not pass for anyone. Immich names dumpsimmich-db-backup-20250729T114018-v1.136.0-pg14.17.sql.gz, so the literalimmich-db-backup-latest.sql.gzcan never exist, and the asserted tables were renamed to singular upstream. Verified against Immich's source: the schema declares@Table('user')and@Table('asset'). Since recipes take literal paths with no globbing, the postgres check is commented out with the shell line that gives the dump a stable name.README and
agent-configuration.mdlisted six read-only subcommands; the closed type has seven.SECURITY.mdalready said seven, so the repo contradicted itself on the one claim a security-minded reader will diff against the source.alerts.mdattributed the hourly cron tovercel.json, which schedules the daily backstop. Billing docs advertised enforced limits while the live site gives everything away during the beta, so repo readers were seeing the worse offer.Verification
Full Go suite,
-race, golangci-lint clean.run.sh,run-docker.sh(now four cases) andrun-s3.shall pass. The corrected cron line andrestic lswere run verbatim.