diff --git a/migrations/20190830204531-tasks-rename-errorMessage-errorJson.js b/migrations/20190830204531-tasks-rename-errorMessage-errorJson.js new file mode 100644 index 000000000..e2ebf5285 --- /dev/null +++ b/migrations/20190830204531-tasks-rename-errorMessage-errorJson.js @@ -0,0 +1,15 @@ +'use strict'; + +exports.up = function(db, callback) { + db.runSql('ALTER TABLE tasks CHANGE errorMessage errorJson TEXT', [], function (error) { + if (error) console.error(error); + callback(error); + }); +}; + +exports.down = function(db, callback) { + db.runSql('ALTER TABLE tasks CHANGE errorJson errorMessage TEXT', [], function (error) { + if (error) console.error(error); + callback(error); + }); +}; diff --git a/migrations/schema.sql b/migrations/schema.sql index c2198c2a5..852d7e82f 100644 --- a/migrations/schema.sql +++ b/migrations/schema.sql @@ -211,8 +211,8 @@ CREATE TABLE IF NOT EXISTS tasks( type VARCHAR(32) NOT NULL, percent INTEGER DEFAULT 0, message TEXT, - errorMessage TEXT, - result TEXT, + errorJson TEXT, + resultJson TEXT, creationTime TIMESTAMP NOT NULL DEFAULT CURRENT_TIMESTAMP, ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP, PRIMARY KEY (id)); diff --git a/src/routes/test/tasks-test.js b/src/routes/test/tasks-test.js index 26b3c3a84..325390241 100644 --- a/src/routes/test/tasks-test.js +++ b/src/routes/test/tasks-test.js @@ -66,7 +66,7 @@ describe('Tasks API', function () { expect(res.body.active).to.be(false); // finished expect(res.body.success).to.be(true); expect(res.body.result).to.be('ping'); - expect(res.body.errorMessage).to.be(null); + expect(res.body.error).to.be(null); done(); }); }); @@ -117,7 +117,7 @@ describe('Tasks API', function () { expect(res.body.active).to.be(false); // finished expect(res.body.success).to.be(false); expect(res.body.result).to.be(null); - expect(res.body.errorMessage).to.contain('signal SIGTERM'); + expect(res.body.error.message).to.contain('signal SIGTERM'); done(); }); }); @@ -148,7 +148,7 @@ describe('Tasks API', function () { expect(res.body.tasks[0].active).to.be(false); // finished expect(res.body.tasks[0].success).to.be(true); // finished expect(res.body.tasks[0].result).to.be('ping'); - expect(res.body.tasks[0].errorMessage).to.be(null); + expect(res.body.tasks[0].error).to.be(null); done(); }); }); diff --git a/src/taskdb.js b/src/taskdb.js index 801e05a9d..c9384d7ce 100644 --- a/src/taskdb.js +++ b/src/taskdb.js @@ -13,7 +13,7 @@ let assert = require('assert'), DatabaseError = require('./databaseerror'), safe = require('safetydance'); -const TASKS_FIELDS = [ 'id', 'type', 'argsJson', 'percent', 'message', 'errorMessage', 'creationTime', 'resultJson', 'ts' ]; +const TASKS_FIELDS = [ 'id', 'type', 'argsJson', 'percent', 'message', 'errorJson', 'creationTime', 'resultJson', 'ts' ]; function postProcess(task) { assert.strictEqual(typeof task, 'object'); @@ -26,6 +26,9 @@ function postProcess(task) { task.result = JSON.parse(task.resultJson); delete task.resultJson; + + task.error = safe.JSON.parse(task.errorJson); + delete task.errorJson; } function add(task, callback) { @@ -50,8 +53,8 @@ function update(id, data, callback) { let args = [ ]; let fields = [ ]; for (let k in data) { - if (k === 'result') { - fields.push('resultJson = ?'); + if (k === 'result' || k === 'error') { + fields.push(`${k}Json = ?`); args.push(JSON.stringify(data[k])); } else { fields.push(k + ' = ?'); diff --git a/src/tasks.js b/src/tasks.js index fc85a6c08..0d2956e62 100644 --- a/src/tasks.js +++ b/src/tasks.js @@ -25,6 +25,10 @@ exports = module.exports = { TASK_CLEAN_BACKUPS: 'cleanBackups', TASK_SYNC_EXTERNAL_LDAP: 'syncExternalLdap', + // error codes + ESTOPPED: 'stopped', + ECRASHED: 'crashed', + // testing _TASK_IDENTITY: '_identity', _TASK_CRASH: '_crash', @@ -45,10 +49,10 @@ let assert = require('assert'), util = require('util'), _ = require('underscore'); -const NOOP_CALLBACK = function (error) { if (error) debug(error); }; - let gTasks = {}; // indexed by task id +const NOOP_CALLBACK = function (error) { if (error) debug(error); }; + function TaskError(reason, errorOrMessage) { assert.strictEqual(typeof reason, 'string'); assert(errorOrMessage instanceof Error || typeof errorOrMessage === 'string' || typeof errorOrMessage === 'undefined'); @@ -76,7 +80,13 @@ function postProcess(result) { assert.strictEqual(typeof result, 'object'); result.active = !!gTasks[result.id]; - result.success = result.percent === 100 && !result.errorMessage; + // we rely on 'percent' to determine success. maybe this can become a db field + result.success = result.percent === 100 && !result.error; + + // the error in db will be empty if we didn't get a chance to handle task exit + if (!result.active && result.percent !== 100 && !result.error) { + result.error = { message: 'Cloudron crashed/stopped', code: exports.ECRASHED }; + } } function get(id, callback) { @@ -139,20 +149,23 @@ function startTask(taskId, options, callback) { debug(`startTask: ${taskId} completed with code ${code} and signal ${signal}`); get(taskId, function (error, task) { + let taskError; if (!error && task.percent !== 100) { // task crashed or was killed by us - error = code === 0 ? new Error(`Task ${taskId} stopped`) : new Error(`Task ${taskId} crashed with code ${code} and signal ${signal}`); - error.stopped = code === 0; - error.crashed = code !== 0; - update(taskId, { percent: 100, errorMessage: error.message }, NOOP_CALLBACK); - } else if (!error && task.errorMessage) { - error = new Error(task.errorMessage); + taskError = { + message: code === 0 ? `Task ${taskId} stopped` : `Task ${taskId} crashed with code ${code} and signal ${signal}`, + code: code === 0 ? exports.ESTOPPED : exports.ECRASHED + }; + // note that despite the update() here, we should handle the case where the box code was restarted and never got taskworker exit + update(taskId, { percent: 100, error: taskError }, NOOP_CALLBACK); + } else if (!error && task.error) { + taskError = task.error; } else if (!task) { // db got cleared in tests - error = new Error(`No such task ${taskId}`); + taskError = new Error(`No such task ${taskId}`); } delete gTasks[taskId]; - callback(error, task ? task.result : null); + callback(taskError, task ? task.result : null); debug(`startTask: ${taskId} done`); }); @@ -245,6 +258,6 @@ function getLogs(taskId, options, callback) { // removes all fields that are strictly private and should never be returned by API calls function removePrivateFields(task) { - var result = _.pick(task, 'id', 'type', 'percent', 'message', 'errorMessage', 'active', 'creationTime', 'result', 'ts', 'success'); + var result = _.pick(task, 'id', 'type', 'percent', 'message', 'error', 'active', 'creationTime', 'result', 'ts', 'success'); return result; } diff --git a/src/taskworker.js b/src/taskworker.js index 1b755747e..9ad4085a8 100755 --- a/src/taskworker.js +++ b/src/taskworker.js @@ -57,7 +57,12 @@ initialize(function (error) { const progressCallback = (progress, cb) => tasks.update(taskId, progress, cb || NOOP_CALLBACK); const resultCallback = (error, result) => { - const progress = { percent: 100, result: result || null, errorMessage: error ? error.message : null }; + // Error object has properties with enumerable: false (https://mattcbaker.com/posts/stringify-javascript-error/) + const progress = { + percent: 100, + result: result || null, + error: error ? JSON.parse(JSON.stringify(error, Object.getOwnPropertyNames(error))) : null + }; tasks.update(taskId, progress, () => process.exit(error ? 50 : 0)); };