tasks: make error a json

also, handle case where we never got to handle task exit cleanly

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();
                     });
             });

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 + ' = ?');

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;
 }

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));
         };