jjkiers/cloudron-box
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

Cleanup app error codes

Browse Source 
1. The error classes (like AppsError) now take a 3rd argument details.
We can attach anything in this 3rd argument and this gets sent in the
REST response as well.

2. The HttpError class is now HttpError(statusCode, errorOrMessage). If
it's an error object, it will take the message and other things which
were attached above from it and send them across. Previously, we used to
mark this case an internal error all the time.

3. AppsError only has generic codes now. The UI code then simply checks
for additional information that we attached to show errors. For example,
BAD_FIELD will have a field: 'xx' indicating which field is at fault.
ALREADY_EXISTS has information on which domain or port caused a problem.
The advantage here is we can drop all these error codes that are
specific to each model code.

4. Maybe some day, we can remove all these error classes and have only
one generic class. AppsError right now is pretty generic already. We can
use that error code everywhere... No need to translate errors also
everywhere.

5. Finally, in the router code, I have this function toHttpError (in
apps.js) which is also so much cleaner than what we have now. We keep
writing the same stuff over and over.
This commit is contained in:
Girish Ramakrishnan
2019-09-03 10:09:57 -07:00
parent a9e101d9f4
commit e117ee2bef
2 changed files with 34 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

71
src/apps.js
View File

@@ -143,7 +143,7 @@ function AppsError(reason, errorOrMessage, details) {
this.nestedError = errorOrMessage;
}
this.details = _.extend({ reason }, details || {});
this.details = details || {};
}
util.inherits(AppsError, Error);
AppsError.INTERNAL_ERROR = 'Internal Error';
@@ -152,10 +152,7 @@ AppsError.ALREADY_EXISTS = 'Already Exists';
AppsError.NOT_FOUND = 'Not Found';
AppsError.BAD_FIELD = 'Bad Field';
AppsError.BAD_STATE = 'Bad State';
AppsError.PORT_CONFLICT = 'Port Conflict';
AppsError.LOCATION_CONFLICT = 'Location Conflict';
AppsError.PLAN_LIMIT = 'Plan Limit';
AppsError.BAD_CERTIFICATE = 'Invalid certificate';
const NOOP_CALLBACK = function (error) { if (error) debug(error); };
@@ -196,12 +193,12 @@ function validatePortBindings(portBindings, manifest) {
if (!portBindings) return null;
for (let portName in portBindings) {
if (!/^[a-zA-Z0-9_]+$/.test(portName)) return new AppsError(AppsError.BAD_FIELD, `${portName} is not a valid environment variable`);
if (!/^[a-zA-Z0-9_]+$/.test(portName)) return new AppsError(AppsError.BAD_FIELD, `${portName} is not a valid environment variable`, { field: 'portBindings', portName: portName });
const hostPort = portBindings[portName];
if (!Number.isInteger(hostPort)) return new AppsError(AppsError.BAD_FIELD, `${hostPort} is not an integer`);
if (RESERVED_PORTS.indexOf(hostPort) !== -1) return new AppsError(AppsError.PORT_CONFLICT, `Port ${hostPort} is reserved.`);
if (hostPort <= 1023 || hostPort > 65535) return new AppsError(AppsError.BAD_FIELD, `${hostPort} is not in permitted range`);
if (!Number.isInteger(hostPort)) return new AppsError(AppsError.BAD_FIELD, `${hostPort} is not an integer`, { field: 'portBindings', portName: portName });
if (RESERVED_PORTS.indexOf(hostPort) !== -1) return new AppsError(AppsError.BAD_FIELD, `Port ${hostPort} is reserved.`, { field: 'portBindings', portName: portName });
if (hostPort <= 1023 || hostPort > 65535) return new AppsError(AppsError.BAD_FIELD, `${hostPort} is not in permitted range`, { field: 'portBindings', portName: portName });
}
// it is OK if there is no 1-1 mapping between values in manifest.tcpPorts and portBindings. missing values implies
@@ -209,7 +206,7 @@ function validatePortBindings(portBindings, manifest) {
const tcpPorts = manifest.tcpPorts || { };
const udpPorts = manifest.udpPorts || { };
for (let portName in portBindings) {
if (!(portName in tcpPorts) && !(portName in udpPorts)) return new AppsError(AppsError.BAD_FIELD, `Invalid portBindings ${portName}`);
if (!(portName in tcpPorts) && !(portName in udpPorts)) return new AppsError(AppsError.BAD_FIELD, `Invalid portBindings ${portName}`, { field: 'portBindings', portName: portName });
}
return null;
@@ -284,7 +281,7 @@ function validateRobotsTxt(robotsTxt) {
if (robotsTxt === null) return null;
// this is the nginx limit on inline strings. if we really hit this, we have to generate a file
if (robotsTxt.length > 4096) return new AppsError(AppsError.BAD_FIELD, 'robotsTxt must be less than 4096');
if (robotsTxt.length > 4096) return new AppsError(AppsError.BAD_FIELD, 'robotsTxt must be less than 4096', { field: 'robotsTxt' });
// TODO: validate the robots file? we escape the string when templating the nginx config right now
@@ -302,26 +299,26 @@ function validateBackupFormat(format) {
function validateLabel(label) {
if (label === null) return null;
if (label.length > 128) return new AppsError(AppsError.BAD_FIELD, 'label must be less than 128');
if (label.length > 128) return new AppsError(AppsError.BAD_FIELD, 'label must be less than 128', { field: 'label' });
return null;
}
function validateTags(tags) {
if (!Array.isArray(tags)) return new AppsError(AppsError.BAD_FIELD, 'tags must be an array of strings');
if (tags.length > 64) return new AppsError(AppsError.BAD_FIELD, 'Can only set up to 64 tags');
if (!Array.isArray(tags)) return new AppsError(AppsError.BAD_FIELD, 'tags must be an array of strings', { field: 'tags' });
if (tags.length > 64) return new AppsError(AppsError.BAD_FIELD, 'Can only set up to 64 tags', { field: 'tags' });
if (tags.some(tag => (!tag || typeof tag !== 'string'))) return new AppsError(AppsError.BAD_FIELD, 'tags must be an array of non-empty strings');
if (tags.some(tag => tag.length > 128)) return new AppsError(AppsError.BAD_FIELD, 'tag must be less than 128');
if (tags.some(tag => (!tag || typeof tag !== 'string'))) return new AppsError(AppsError.BAD_FIELD, 'tags must be an array of non-empty strings', { field: 'tags' });
if (tags.some(tag => tag.length > 128)) return new AppsError(AppsError.BAD_FIELD, 'tag must be less than 128', { field: 'tags' });
return null;
}
function validateEnv(env) {
for (let key in env) {
if (key.length > 512) return new AppsError(AppsError.BAD_FIELD, 'Max env var key length is 512');
if (key.length > 512) return new AppsError(AppsError.BAD_FIELD, 'Max env var key length is 512', { field: 'env', env: env });
// http://pubs.opengroup.org/onlinepubs/000095399/basedefs/xbd_chap08.html
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) return new AppsError(AppsError.BAD_FIELD, `"${key}" is not a valid environment variable`);
if (!/^[a-zA-Z_][a-zA-Z0-9_]*$/.test(key)) return new AppsError(AppsError.BAD_FIELD, `"${key}" is not a valid environment variable`, { field: 'env', env: env });
}
return null;
@@ -330,23 +327,23 @@ function validateEnv(env) {
function validateDataDir(dataDir) {
if (dataDir === '') return null; // revert back to default dataDir
if (path.resolve(dataDir) !== dataDir) return new AppsError(AppsError.BAD_FIELD, 'dataDir must be an absolute path');
if (path.resolve(dataDir) !== dataDir) return new AppsError(AppsError.BAD_FIELD, 'dataDir must be an absolute path', { field: 'dataDir' });
// nfs shares will have the directory mounted already
let stat = safe.fs.lstatSync(dataDir);
if (stat) {
if (!stat.isDirectory()) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} is not a directory`);
if (!stat.isDirectory()) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} is not a directory`, { field: 'dataDir' });
let entries = safe.fs.readdirSync(dataDir);
if (!entries) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} could not be listed`);
if (entries.length !== 0) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} is not empty`);
if (!entries) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} could not be listed`, { field: 'dataDir' });
if (entries.length !== 0) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} is not empty`, { field: 'dataDir' });
}
// backup logic relies on paths not overlapping (because it recurses)
if (dataDir.startsWith(paths.APPS_DATA_DIR)) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} cannot be inside apps data`);
if (dataDir.startsWith(paths.APPS_DATA_DIR)) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} cannot be inside apps data`, { field: 'dataDir' });
// if we made it this far, it cannot start with any of these realistically
const fhs = [ '/bin', '/boot', '/etc', '/lib', '/lib32', '/lib64', '/proc', '/run', '/sbin', '/tmp', '/usr' ];
if (fhs.some((p) => dataDir.startsWith(p))) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} cannot be placed inside this location`);
if (fhs.some((p) => dataDir.startsWith(p))) return new AppsError(AppsError.BAD_FIELD, `dataDir ${dataDir} cannot be placed inside this location`, { field: 'dataDir' });
return null;
}
@@ -368,19 +365,19 @@ function getDuplicateErrorDetails(errorMessage, location, domainObject, portBind
if (match[2] === 'subdomain') {
// mysql reports a unique conflict with a dash: eg. domain:example.com subdomain:test => test-example.com
if (match[1] === `${location}-${domainObject.domain}`) {
return new AppsError(AppsError.LOCATION_CONFLICT, `Domain '${domains.fqdn(location, domainObject)}' is in use`, { location: location, domain: domainObject.domain });
return new AppsError(AppsError.ALREADY_EXISTS, `Domain '${domains.fqdn(location, domainObject)}' is in use`, { location: location, domain: domainObject.domain });
}
for (let d of alternateDomains) {
if (match[1] !== `${d.subdomain}-${d.domain}`) continue;
return new AppsError(AppsError.LOCATION_CONFLICT, `Alternate domain '${d.subdomain}.${d.domain}' is in use`, { location: d.subdomain, domain: d.domain });
return new AppsError(AppsError.ALREADY_EXISTS, `Alternate domain '${d.subdomain}.${d.domain}' is in use`, { location: d.subdomain, domain: d.domain });
}
}
// check if any of the port bindings conflict
for (let portName in portBindings) {
if (portBindings[portName] === parseInt(match[1])) return new AppsError(AppsError.PORT_CONFLICT, `Port ${match[1]} is reserved`, { portName });
if (portBindings[portName] === parseInt(match[1])) return new AppsError(AppsError.ALREADY_EXISTS, `Port ${match[1]} is reserved`, { portName });
}
return new AppsError(AppsError.ALREADY_EXISTS, `${match[2]} '${match[1]}' is in use`);
@@ -638,7 +635,7 @@ function scheduleTask(appId, args, values, callback) {
appdb.setTask(appId, values, function (error) {
if (error && error.reason === DatabaseError.ALREADY_EXISTS) return callback(new AppsError(AppsError.ALREADY_EXISTS, error.message));
if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.BAD_STATE)); // could be because app went away OR a taskId exists
if (error && error.reason === DatabaseError.NOT_FOUND) return callback(new AppsError(AppsError.BAD_STATE, 'Another task is scheduled for this app')); // could be because app went away OR a taskId exists
if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, error));
appTaskManager.scheduleTask(appId, taskId, function (error) {
@@ -728,7 +725,7 @@ function install(data, user, auditSource, callback) {
if (mailboxName) {
error = mail.validateName(mailboxName);
if (error) return callback(error);
if (error) return callback(new AppsError(AppsError.BAD_FIELD, error.message, { field: 'mailboxName' }));
} else {
mailboxName = mailboxNameForLocation(location, manifest);
}
@@ -736,7 +733,7 @@ function install(data, user, auditSource, callback) {
var appId = uuid.v4();
if (icon) {
if (!validator.isBase64(icon)) return callback(new AppsError(AppsError.BAD_FIELD, 'icon is not base64'));
if (!validator.isBase64(icon)) return callback(new AppsError(AppsError.BAD_FIELD, 'icon is not base64', { field: 'icon' }));
if (!safe.fs.writeFileSync(path.join(paths.APP_ICONS_DIR, appId + '.png'), Buffer.from(icon, 'base64'))) {
return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Error saving icon:' + safe.error.message));
@@ -748,11 +745,11 @@ function install(data, user, auditSource, callback) {
if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Could not get domain info:' + error.message));
error = domains.validateHostname(location, domainObject);
if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Bad location: ' + error.message));
if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Bad location: ' + error.message, { field: 'location' }));
if (cert && key) {
error = reverseProxy.validateCertificate(location, domainObject, { cert, key });
if (error) return callback(new AppsError(AppsError.BAD_CERTIFICATE, error.message));
if (error) return callback(new AppsError(AppsError.BAD_FIELD, error.message, { field: 'cert' }));
}
debug('Will install app with id : ' + appId);
@@ -856,7 +853,7 @@ function configure(appId, data, user, auditSource, callback) {
if ('mailboxName' in data) {
if (data.mailboxName) {
error = mail.validateName(data.mailboxName);
if (error) return callback(error);
if (error) return callback(new AppsError(AppsError.BAD_FIELD, error.message, { field: 'mailboxName' }));
values.mailboxName = data.mailboxName;
} else {
values.mailboxName = mailboxNameForLocation(location, app.manifest);
@@ -896,7 +893,7 @@ function configure(appId, data, user, auditSource, callback) {
if ('icon' in data) {
if (data.icon) {
if (!validator.isBase64(data.icon)) return callback(new AppsError(AppsError.BAD_FIELD, 'icon is not base64'));
if (!validator.isBase64(data.icon)) return callback(new AppsError(AppsError.BAD_FIELD, 'icon is not base64', { field: 'icon' }));
if (!safe.fs.writeFileSync(path.join(paths.APP_ICONS_DIR, appId + '.user.png'), Buffer.from(data.icon, 'base64'))) {
return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Error saving icon:' + safe.error.message));
@@ -911,13 +908,13 @@ function configure(appId, data, user, auditSource, callback) {
if (error) return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Could not get domain info:' + error.message));
error = domains.validateHostname(location, domainObject);
if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Bad location: ' + error.message));
if (error) return callback(new AppsError(AppsError.BAD_FIELD, 'Bad location: ' + error.message, { field: 'location' }));
// save cert to boxdata/certs. TODO: move this to apptask when we have a real task queue
if ('cert' in data && 'key' in data) {
if (data.cert && data.key) {
error = reverseProxy.validateCertificate(location, domainObject, { cert: data.cert, key: data.key });
if (error) return callback(new AppsError(AppsError.BAD_CERTIFICATE, error.message));
if (error) return callback(new AppsError(AppsError.BAD_FIELD, error.message, { field: 'cert' }));
}
error = reverseProxy.setAppCertificateSync(location, domainObject, { cert: data.cert, key: data.key });
@@ -986,7 +983,7 @@ function update(appId, data, auditSource, callback) {
if ('icon' in data) {
if (data.icon) {
if (!validator.isBase64(data.icon)) return callback(new AppsError(AppsError.BAD_FIELD, 'icon is not base64'));
if (!validator.isBase64(data.icon)) return callback(new AppsError(AppsError.BAD_FIELD, 'icon is not base64', { field: 'icon' }));
if (!safe.fs.writeFileSync(path.join(paths.APP_ICONS_DIR, appId + '.user.png'), Buffer.from(data.icon, 'base64'))) {
return callback(new AppsError(AppsError.INTERNAL_ERROR, 'Error saving icon:' + safe.error.message));
@@ -1180,7 +1177,7 @@ function clone(appId, data, user, auditSource, callback) {
if (mailboxName) {
error = mail.validateName(mailboxName);
if (error) return callback(error);
if (error) return callback(new AppsError(AppsError.BAD_FIELD, error.message, { field: 'mailboxName' }));
} else {
mailboxName = mailboxNameForLocation(location, manifest);
}