apps: onFinished handler not called across restarts

if box code restarts in the middle of a apptask, the onFinished handlers
are not called for data migration and update. rework the code to hook
the onFinished handlers when the task completes and not where the task
is started.
This commit is contained in:
Girish Ramakrishnan
2021-09-30 10:31:50 -07:00
parent 445c83c8b9
commit b0bdfbd870

View File

@@ -140,6 +140,7 @@ exports = module.exports = {
const appstore = require('./appstore.js'), const appstore = require('./appstore.js'),
appTaskManager = require('./apptaskmanager.js'), appTaskManager = require('./apptaskmanager.js'),
assert = require('assert'), assert = require('assert'),
AuditSource = require('./auditsource.js'),
backups = require('./backups.js'), backups = require('./backups.js'),
BoxError = require('./boxerror.js'), BoxError = require('./boxerror.js'),
constants = require('./constants.js'), constants = require('./constants.js'),
@@ -1029,11 +1030,33 @@ function mailboxNameForLocation(location, manifest) {
return 'noreply.app'; return 'noreply.app';
} }
async function scheduleTask(appId, installationState, taskId, onFinished) { async function onTaskFinished(appId, installationState, taskId, error) {
const success = !error;
const errorMessage = error?.message || null;
switch (installationState) {
case exports.ISTATE_PENDING_DATA_DIR_MIGRATION:
if (success) await safe(services.rebuildService('sftp', AuditSource.APPTASK), { debug });
break;
case exports.ISTATE_PENDING_UPDATE: {
const app = await get(appId);
if (!app) break;
const task = await tasks.get(taskId);
if (!task) break;
const fromManifest = success ? task.args[1].updateConfig.manifest : app.manifest;
const toManifest = success ? app.manifest : task.args[1].updateConfig.manifest;
await eventlog.add(eventlog.ACTION_APP_UPDATE_FINISH, AuditSource.APPTASK, { app, toManifest, fromManifest, success, errorMessage });
break;
}
}
}
async function scheduleTask(appId, installationState, taskId) {
assert.strictEqual(typeof appId, 'string'); assert.strictEqual(typeof appId, 'string');
assert.strictEqual(typeof installationState, 'string'); assert.strictEqual(typeof installationState, 'string');
assert.strictEqual(typeof taskId, 'string'); assert.strictEqual(typeof taskId, 'string');
assert(typeof onFinished === 'undefined' || typeof onFinished === 'function');
const backupConfig = await settings.getBackupConfig(); const backupConfig = await settings.getBackupConfig();
@@ -1057,12 +1080,12 @@ async function scheduleTask(appId, installationState, taskId, onFinished) {
// see also apptask makeTaskError // see also apptask makeTaskError
boxError.details.taskId = taskId; boxError.details.taskId = taskId;
boxError.details.installationState = installationState; boxError.details.installationState = installationState;
await safe(update(appId, { installationState: exports.ISTATE_ERROR, error: boxError.toPlainObject(), taskId: null })); await safe(update(appId, { installationState: exports.ISTATE_ERROR, error: boxError.toPlainObject(), taskId: null }), { debug });
} else if (!(installationState === exports.ISTATE_PENDING_UNINSTALL && !error)) { // clear out taskId except for successful uninstall } else if (!(installationState === exports.ISTATE_PENDING_UNINSTALL && !error)) { // clear out taskId except for successful uninstall
await safe(update(appId, { taskId: null })); await safe(update(appId, { taskId: null }), { debug });
} }
if (onFinished) onFinished(error); await safe(onTaskFinished(appId, installationState, taskId, error), { debug }); // ignore error
}); });
} }
@@ -1083,7 +1106,7 @@ async function addTask(appId, installationState, task) {
if (updateError && updateError.reason === BoxError.NOT_FOUND) throw new BoxError(BoxError.BAD_STATE, 'Another task is scheduled for this app'); // could be because app went away OR a taskId exists if (updateError && updateError.reason === BoxError.NOT_FOUND) throw new BoxError(BoxError.BAD_STATE, 'Another task is scheduled for this app'); // could be because app went away OR a taskId exists
if (updateError) throw updateError; if (updateError) throw updateError;
if (scheduleNow) await safe(scheduleTask(appId, installationState, taskId, task.onFinished)); // ignore error if (scheduleNow) await safe(scheduleTask(appId, installationState, taskId), { debug }); // ignore error
return taskId; return taskId;
} }
@@ -1627,10 +1650,7 @@ async function setDataDir(app, dataDir, auditSource) {
const task = { const task = {
args: { newDataDir: dataDir }, args: { newDataDir: dataDir },
values: { }, values: {}
onFinished: async (error) => {
if (!error) await safe(services.rebuildService('sftp', auditSource), { debug });
}
}; };
const taskId = await addTask(appId, exports.ISTATE_PENDING_DATA_DIR_MIGRATION, task); const taskId = await addTask(appId, exports.ISTATE_PENDING_DATA_DIR_MIGRATION, task);
@@ -1704,10 +1724,7 @@ async function updateApp(app, data, auditSource) {
const task = { const task = {
args: { updateConfig }, args: { updateConfig },
values, values
onFinished: (error) => {
eventlog.add(eventlog.ACTION_APP_UPDATE_FINISH, auditSource, { app, toManifest: manifest, fromManifest: app.manifest, success: !error, errorMessage: error ? error.message : null });
}
}; };
const taskId = await addTask(appId, exports.ISTATE_PENDING_UPDATE, task); const taskId = await addTask(appId, exports.ISTATE_PENDING_UPDATE, task);
@@ -2378,7 +2395,7 @@ async function schedulePendingTasks() {
debug(`schedulePendingTasks: schedule task for ${app.fqdn} ${app.id}: state=${app.installationState},taskId=${app.taskId}`); debug(`schedulePendingTasks: schedule task for ${app.fqdn} ${app.id}: state=${app.installationState},taskId=${app.taskId}`);
await safe(scheduleTask(app.id, app.installationState, app.taskId)); // ignore error await safe(scheduleTask(app.id, app.installationState, app.taskId), { debug }); // ignore error
} }
} }