Exit non-zero on backup/verify failures + optional post-run hooks - #10
Conversation
…js#6) backup exits 1 when any asset fails to upload; verify exits 1 on missing or errored objects. New --on-success / --on-failure options run a shell command with the outcome in ATTIC_* environment variables, so unattended launchd runs can alert without log scraping.
| unavailableStore: FileUnavailableAssetStore(), | ||
| adaptiveController: adaptiveController, | ||
| ) | ||
| } catch { |
There was a problem hiding this comment.
warning: This catch only surrounds runBackup. Failures from ensureManifestMigrated, config/keychain setup, manifest loading, asset loading, staging-directory creation, or permission preflight all occur before it, so --on-failure does not run for the documented early config/network/S3 failures. I verified the missing-config path with an isolated empty home: it exited 1 without running the hook. Extend the failure boundary to cover setup as well, while ensuring the report-failure path invokes the hook only once.
There was a problem hiding this comment.
Came to think of it: Failure is not necessarily boolean. If you want to back up 10.000 items, and 9.999 succeed but one fail and is put on a retry queue, the backup task shuold not exit with 1==FAILURE, and should (maybe) not trigger an --on-failure hook. It's more granular.
What do you think about a design where we have three different failure exit codes::
0— clean: everything attempted succeeded.3— partial, recoverable: some assets failed, but every failure is queued for retry and none has stalled; subsequent runs are expected to converge without intervention.6— partial, non-recoverable: at least one asset can never be backed up (permanently unavailable), or has kept failing acrossstalledAttemptThresholdruns — retrying is no longer a plan. Forverify: objects missing from S3.9— fatal: the run aborted before completing (config, keychain, network, S3, permissions).
The --on-success hook would be called for 0. Perhaps also for 3, but with a metadata with an ATTIC_FAILED_SOFT environment count. For 6 and 9 the --on-failure hook is called, but also reporting success counts. Or perhaps there should be only a --post-hook regardless of result and let it be up to the hook script to decide what to do in each case.
| )) | ||
| return nil | ||
| } | ||
| process.waitUntilExit() |
There was a problem hiding this comment.
warning: waitUntilExit() is unbounded. A hung notifier (including the documented curl example when a network request stalls) leaves the attic process running forever, so launchd never receives attic’s success/failure exit status despite the stated guarantee that a broken notifier cannot mask it. Add a finite hook timeout with termination and a warning, or use a clearly documented detached policy, so the backup’s own outcome is always reported.
There was a problem hiding this comment.
Will apply a 60s timeout
Review feedback on tijs#10: the catch only wrapped runBackup, so failures during setup (config, keychain, manifest load, library scan, staging dir, Photos permission preflight) exited 1 without firing the hook — contradicting the documented behavior that --on-failure also fires on early config/network/S3 errors. Move the whole setup + backup body into executeBackup() and make run() a thin outcome wrapper, so any throw fires the failure hook exactly once before the error propagates.
Review feedback on tijs#10: waitUntilExit() was unbounded, so a hung notifier (e.g. a curl stalled on a dead network) kept attic alive forever and launchd never saw the backup's own exit status. Wait on a termination-handler semaphore with a 60s ceiling; on timeout SIGTERM the hook, escalate to SIGKILL after a 5s grace, warn on stderr, and return nil so the backup outcome is always reported. Timeout is a parameter so tests exercise the path in ~0.5s.
…failure-signalling # Conflicts: # CHANGELOG.md
…ilure-signalling # Conflicts: # CHANGELOG.md
2 participants
Fixes #6.
Makes unattended runs monitorable by exit code and adds an optional alerting hook.
Exit codes
backupexits 1 when any asset fails to upload (summary still prints first); fatal errors propagate as beforeverifyexits 1 when objects are missing or errored — it had the same always-zero behavior, which matters for scheduled integrity checksHooks
--on-success/--on-failure <command>run viash -cwith the outcome in env vars:ATTIC_RESULTplusATTIC_UPLOADED/ATTIC_FAILED/ATTIC_SKIPPED/ATTIC_TOTAL_BYTESwhen the run completed--on-failurealso fires when the run aborts early (config/network/S3) with onlyATTIC_RESULTsetHook runner lives in AtticCore (
PostRunHook) with 5 tests (env delivery, exit-status passthrough, unlaunchable shell → warning + nil). Full suite 282/282, swiftlint + swiftformat clean.docs/unattended-backups.mdgains a "Failure signalling and hooks" section with a healthchecks.io launchd example.Drafted with AI assistance; reviewed and tested locally against main.