Summary
When a migration step throws, migrate() resolves successfully rather than rejecting. The error is logged and the journal is rolled back, but there is no programmatic signal (thrown error or error-state return value) for the caller to detect the failure. Consumers therefore continue as if the migration succeeded.
Where
In lib/Task.js:
Task.run catches a throwing step, records it on context.errors, sets context.hasErrored = true, rolls the journal back, and — finding no error-type handler step — lets run() return normally:
} catch (err) {
logger.error(`Task -- shouldContinue errored ${err}`)
this.context.errors.push(err)
this.context.hasErrored = true
journal.undoToIndex(lastJournalEntryIndex)
stepIndex = this.getImmediateNextIndexOfType(stepIndex, 'error')
shouldContinue = (stepIndex !== -1)
if (shouldContinue) continue
}
Task.runApplicable then sees hasErrored, rolls back, and breaks without re-throwing:
const { hasErrored } = await task.run({ cwd, journal, logger })
if (hasErrored) {
journal.undoToIndex(lastJournalEntryIndex)
break // swallowed: no throw, no error return
}So migrate() resolves and journal.data.content is the rolled-back (un-migrated) content — indistinguishable from "no changes were needed".
Impact
Downstream consumers cannot tell a migration failed. In adapt-authoring-adaptframework, course import and framework-update both wrap migration in try/catch to abort on failure, but the catch never fires because the error is swallowed here — a failed migration is silently treated as a successful import.
Suggested fix
When a task has errored (after rollback), surface it to the caller rather than only logging — e.g. re-throw (an AggregateError of context.errors) from Task.runApplicable, or return an error-state result that migrate() can act on. The collected context.errors already carry the necessary detail.
Workaround in place downstream
adapt-authoring-adaptframework currently detects the failure by capturing error-level logs emitted during migrate() and throwing if any occurred. This is a stopgap that should be removed once the error is propagated here.
Summary
When a migration step throws,
migrate()resolves successfully rather than rejecting. The error is logged and the journal is rolled back, but there is no programmatic signal (thrown error or error-state return value) for the caller to detect the failure. Consumers therefore continue as if the migration succeeded.Where
In
lib/Task.js:Task.runcatches a throwing step, records it oncontext.errors, setscontext.hasErrored = true, rolls the journal back, and — finding noerror-type handler step — letsrun()return normally:Task.runApplicablethen seeshasErrored, rolls back, andbreaks without re-throwing:So
migrate()resolves andjournal.data.contentis the rolled-back (un-migrated) content — indistinguishable from "no changes were needed".Impact
Downstream consumers cannot tell a migration failed. In
adapt-authoring-adaptframework, course import and framework-update both wrap migration intry/catchto abort on failure, but thecatchnever fires because the error is swallowed here — a failed migration is silently treated as a successful import.Suggested fix
When a task has errored (after rollback), surface it to the caller rather than only logging — e.g. re-throw (an
AggregateErrorofcontext.errors) fromTask.runApplicable, or return an error-state result thatmigrate()can act on. The collectedcontext.errorsalready carry the necessary detail.Workaround in place downstream
adapt-authoring-adaptframeworkcurrently detects the failure by capturing error-level logs emitted duringmigrate()and throwing if any occurred. This is a stopgap that should be removed once the error is propagated here.