run() called log.Fatalf on the first migration that failed, which ended the
process from inside the migration engine. Nothing above it could record what
happened - an installer needs to write down which version an attempt stopped
on - and no test could exercise a failing migration at all without taking the
test binary with it, which is why the one test that covers a failed migration
drove the registered function directly and left the scheduler uncovered.
run(), Migrate() and MigrateApp() now return an error, and the exit moved to
the command layer where the exit code is the command's business.
Two of those errors say more than "it failed". A migration that fails comes
back as a *VersionFailure naming the version, because an installer records
that as a diagnostic snapshot - the authoritative answer to where a retry
resumes is always recomputed from sys_migration, never read back, and asking
the database what is still pending answers a different question that merely
has the same answer most of the time. An app code nothing registered under is
now an error rather than a log line, so an installer asking for one app by
name cannot be told that installing an app that does not exist succeeded; the
command layer still rejects a typo before any database work.
exitOnError is what makes the command exit non-zero, and it covers more than
it replaces. Every path out of migrateModel used to return without an exit
code: an unreachable tenant database or a failed AutoMigrate printed a line
and exited 0, so a caller that migrates before starting a server - the deploy
workflow does exactly that - carried on onto a schema that had not been
brought forward. A failing migration function was the only failure reported,
and only as a side effect of the log.Fatalf this commit removes.
Each of these was checked by degrading it and watching the named assertion
go red: returning nil instead of the failure, naming the first version rather
than the one that failed, accepting an unregistered app code, and not exiting.
One gap is left open deliberately. Go allows a call whose only result is an
error to stand as a statement, so `migration.Migrate.Migrate()` still compiles
while dropping what it returns - `go build` passed while migrateModel was
doing exactly that during this change. Both call sites now return the value,
which the compiler does check, but nothing guards against the statement form
coming back. A checksilent rule was considered and dropped: that tool parses
without type information, so it could only match the method name, and a guard
that fires on any type with a Migrate method is noise.
