From e695a57853d3ea31605343f2e06e78f090bf47d0 Mon Sep 17 00:00:00 2001 From: Rory Powell Date: Wed, 16 Mar 2022 17:29:47 +0000 Subject: [PATCH] Better error handling around license errors --- packages/backend-core/src/errors/index.js | 26 +++++++++++++++++++ packages/backend-core/src/errors/licensing.js | 10 +++++++ .../backend-core/src/objectStore/index.js | 25 +++++++++++------- packages/server/src/api/index.js | 3 +++ packages/server/src/middleware/usageQuota.ts | 2 +- packages/worker/src/api/index.js | 16 +----------- 6 files changed, 56 insertions(+), 26 deletions(-) diff --git a/packages/backend-core/src/errors/index.js b/packages/backend-core/src/errors/index.js index 139dd5a5a1..429aa68cad 100644 --- a/packages/backend-core/src/errors/index.js +++ b/packages/backend-core/src/errors/index.js @@ -8,8 +8,34 @@ const types = { ...licensing.types, } +const context = { + ...licensing.context, +} + +const getPublicError = err => { + let error + if (err.code || err.type) { + // add generic error information + error = { + code: err.code, + type: err.type, + } + + if (err.code) { + error = { + ...error, + // get any additional context from this error + ...context[err.code](err), + } + } + } + + return error +} + module.exports = { codes, types, UsageLimitError: licensing.UsageLimitError, + getPublicError, } diff --git a/packages/backend-core/src/errors/licensing.js b/packages/backend-core/src/errors/licensing.js index 111f21ad46..c05f9c561e 100644 --- a/packages/backend-core/src/errors/licensing.js +++ b/packages/backend-core/src/errors/licensing.js @@ -8,15 +8,25 @@ const codes = { USAGE_LIMIT_EXCEEDED: "usage_limit_exceeded", } +const context = { + [codes.USAGE_LIMIT_EXCEEDED]: err => { + return { + limitName: err.limitName, + } + }, +} + class UsageLimitError extends BudibaseError { constructor(message, limitName) { super(message, types.LICENSE_ERROR, codes.USAGE_LIMIT_EXCEEDED) this.limitName = limitName + this.status = 400 } } module.exports = { types, codes, + context, UsageLimitError, } diff --git a/packages/backend-core/src/objectStore/index.js b/packages/backend-core/src/objectStore/index.js index b5d8475cee..2385149f4d 100644 --- a/packages/backend-core/src/objectStore/index.js +++ b/packages/backend-core/src/objectStore/index.js @@ -78,6 +78,7 @@ exports.ObjectStore = bucket => { const config = { s3ForcePathStyle: true, signatureVersion: "v4", + apiVersion: "2006-03-01", params: { Bucket: sanitizeBucket(bucket), }, @@ -102,17 +103,21 @@ exports.makeSureBucketExists = async (client, bucketName) => { .promise() } catch (err) { const promises = STATE.bucketCreationPromises + const doesntExist = err.statusCode === 404, + noAccess = err.statusCode === 403 if (promises[bucketName]) { await promises[bucketName] - } else if (err.statusCode === 404) { - // bucket doesn't exist create it - promises[bucketName] = client - .createBucket({ - Bucket: bucketName, - }) - .promise() - await promises[bucketName] - delete promises[bucketName] + } else if (doesntExist || noAccess) { + if (doesntExist) { + // bucket doesn't exist create it + promises[bucketName] = client + .createBucket({ + Bucket: bucketName, + }) + .promise() + await promises[bucketName] + delete promises[bucketName] + } // public buckets are quite hidden in the system, make sure // no bucket is set accidentally if (PUBLIC_BUCKETS.includes(bucketName)) { @@ -124,7 +129,7 @@ exports.makeSureBucketExists = async (client, bucketName) => { .promise() } } else { - throw err + throw new Error("Unable to write to object store bucket.") } } } diff --git a/packages/server/src/api/index.js b/packages/server/src/api/index.js index d5a4867291..0a1509d3cf 100644 --- a/packages/server/src/api/index.js +++ b/packages/server/src/api/index.js @@ -5,6 +5,7 @@ const { buildTenancyMiddleware, buildAppTenancyMiddleware, } = require("@budibase/backend-core/auth") +const { errors } = require("@budibase/backend-core") const currentApp = require("../middleware/currentapp") const compress = require("koa-compress") const zlib = require("zlib") @@ -64,10 +65,12 @@ router.use(async (ctx, next) => { await next() } catch (err) { ctx.status = err.status || err.statusCode || 500 + const error = errors.getPublicError(err) ctx.body = { message: err.message, status: ctx.status, validationErrors: err.validation, + error, } if (env.NODE_ENV !== "jest") { ctx.log.error(err) diff --git a/packages/server/src/middleware/usageQuota.ts b/packages/server/src/middleware/usageQuota.ts index 499d4dd646..835f669a58 100644 --- a/packages/server/src/middleware/usageQuota.ts +++ b/packages/server/src/middleware/usageQuota.ts @@ -134,7 +134,7 @@ const appPreDelete = async (ctx: any, usageContext: any) => { const appPostDelete = async (ctx: any, usageContext: any) => { // delete the app rows from usage - const rowCount = usageContext[Pro.StaticQuotaName.ROWS].rowCount + const rowCount = usageContext[Pro.StaticQuotaName.APPS].rowCount if (rowCount) { await Pro.Licensing.Quotas.updateUsage( -rowCount, diff --git a/packages/worker/src/api/index.js b/packages/worker/src/api/index.js index f2d1047764..25899e2245 100644 --- a/packages/worker/src/api/index.js +++ b/packages/worker/src/api/index.js @@ -113,21 +113,7 @@ router.use(async (ctx, next) => { } catch (err) { ctx.log.error(err) ctx.status = err.status || err.statusCode || 500 - - let error - if (err.code || err.type) { - // add generic error information - error = { - code: err.code, - type: err.type, - } - - // add specific error information - if (error.code === errors.codes.USAGE_LIMIT_EXCEEDED) { - error.limitName = err.limitName - } - } - + const error = errors.getPublicError(err) ctx.body = { message: err.message, status: ctx.status,