From 292d520b30a7ab887c3054d61dc02bde8d53741a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 14:54:47 +0100 Subject: [PATCH 1/9] Adding some changes for to redis library, allowing reconnection. --- hosting/docker-compose.yaml | 2 + packages/auth/src/redis/index.js | 95 +++++++++++-------- packages/worker/src/api/routes/admin/users.js | 9 +- 3 files changed, 68 insertions(+), 38 deletions(-) diff --git a/hosting/docker-compose.yaml b/hosting/docker-compose.yaml index 37657ce009..0cd7bc92bf 100644 --- a/hosting/docker-compose.yaml +++ b/hosting/docker-compose.yaml @@ -105,6 +105,8 @@ services: restart: always image: redis command: redis-server --requirepass ${REDIS_PASSWORD} + ports: + - "${REDIS_PORT}:6379" volumes: - redis_data:/data diff --git a/packages/auth/src/redis/index.js b/packages/auth/src/redis/index.js index 78e3ea7acd..0fd0aa8c83 100644 --- a/packages/auth/src/redis/index.js +++ b/packages/auth/src/redis/index.js @@ -3,40 +3,64 @@ const env = require("../environment") const Redis = env.isTest() ? require("ioredis-mock") : require("ioredis") const { addDbPrefix, removeDbPrefix, getRedisOptions } = require("./utils") +const RETRY_PERIOD_MS = 2000 +const MAX_RETRIES = 20 const CLUSTERED = false // for testing just generate the client once let CONNECTED = false let CLIENT = env.isTest() ? new Redis(getRedisOptions()) : null +function retryConnection() { + setTimeout(init, RETRY_PERIOD_MS) +} + /** * Inits the system, will error if unable to connect to redis cluster (may take up to 10 seconds) otherwise * will return the ioredis client which will be ready to use. - * @return {Promise} The ioredis client. */ function init() { - return new Promise((resolve, reject) => { - // testing uses a single in memory client - if (env.isTest() || (CLIENT && CONNECTED)) { - return resolve(CLIENT) + function errorOccurred(err) { + CONNECTED = false; + console.error("Redis connection failed - " + err) + setTimeout(() => { + init() + }, RETRY_PERIOD_MS) + } + // testing uses a single in memory client + if (env.isTest() || (CLIENT && CONNECTED)) { + return + } + if (CLIENT) { + CLIENT.disconnect() + } + const { opts, host, port } = getRedisOptions(CLUSTERED) + if (CLUSTERED) { + CLIENT = new Redis.Cluster([{ host, port }], opts) + } else { + CLIENT = new Redis(opts) + } + CLIENT.on("end", err => { + errorOccurred(err) + }) + CLIENT.on("error", err => { + errorOccurred(err) + }) + CLIENT.on("connect", () => { + CONNECTED = true + }) +} + +function waitForConnection() { + return new Promise(resolve => { + if (CLIENT == null) { + init() + } else if (CONNECTED) { + resolve() + return } - const { opts, host, port } = getRedisOptions(CLUSTERED) - if (CLUSTERED) { - CLIENT = new Redis.Cluster([{ host, port }], opts) - } else { - CLIENT = new Redis(opts) - } - CLIENT.on("end", err => { - reject(err) - CONNECTED = false - }) - CLIENT.on("error", err => { - reject(err) - CONNECTED = false - }) CLIENT.on("connect", () => { - resolve(CLIENT) - CONNECTED = true + resolve() }) }) } @@ -85,31 +109,30 @@ class RedisWrapper { } async init() { - this._client = await init() + init() + await waitForConnection() return this } async finish() { - this._client.disconnect() + CLIENT.disconnect() } async scan() { - const db = this._db, - client = this._client + const db = this._db let stream if (CLUSTERED) { - let node = client.nodes("master") + let node = CLIENT.nodes("master") stream = node[0].scanStream({ match: db + "-*", count: 100 }) } else { - stream = client.scanStream({ match: db + "-*", count: 100 }) + stream = CLIENT.scanStream({ match: db + "-*", count: 100 }) } return promisifyStream(stream) } async get(key) { - const db = this._db, - client = this._client - let response = await client.get(addDbPrefix(db, key)) + const db = this._db + let response = await CLIENT.get(addDbPrefix(db, key)) // overwrite the prefixed key if (response != null && response.key) { response.key = key @@ -123,22 +146,20 @@ class RedisWrapper { } async store(key, value, expirySeconds = null) { - const db = this._db, - client = this._client + const db = this._db if (typeof value === "object") { value = JSON.stringify(value) } const prefixedKey = addDbPrefix(db, key) - await client.set(prefixedKey, value) + await CLIENT.set(prefixedKey, value) if (expirySeconds) { - await client.expire(prefixedKey, expirySeconds) + await CLIENT.expire(prefixedKey, expirySeconds) } } async delete(key) { - const db = this._db, - client = this._client - await client.del(addDbPrefix(db, key)) + const db = this._db + await CLIENT.del(addDbPrefix(db, key)) } async clear() { diff --git a/packages/worker/src/api/routes/admin/users.js b/packages/worker/src/api/routes/admin/users.js index f334f05e7d..cfff67f3e7 100644 --- a/packages/worker/src/api/routes/admin/users.js +++ b/packages/worker/src/api/routes/admin/users.js @@ -6,6 +6,13 @@ const Joi = require("joi") const router = Router() +function buildAdminInitValidation() { + return joiValidator.body(Joi.object({ + email: Joi.string().required(), + password: Joi.string().required(), + }).required().unknown(false)) +} + function buildUserSaveValidation(isSelf = false) { let schema = { email: Joi.string().allow(null, ""), @@ -74,7 +81,7 @@ router buildInviteAcceptValidation(), controller.inviteAccept ) - .post("/api/admin/users/init", controller.adminUser) + .post("/api/admin/users/init", buildAdminInitValidation(), controller.adminUser) .get("/api/admin/users/self", controller.getSelf) // admin endpoint but needs to come at end (blocks other endpoints otherwise) .get("/api/admin/users/:id", adminOnly, controller.find) From 428a9e5ba3266abcda59d7f49cc89ef32e4ec83c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 14:56:23 +0100 Subject: [PATCH 2/9] Formatting. --- packages/auth/src/redis/index.js | 2 +- packages/worker/src/api/routes/admin/users.js | 18 +++++++++++++----- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/packages/auth/src/redis/index.js b/packages/auth/src/redis/index.js index 0fd0aa8c83..7a1b9a3431 100644 --- a/packages/auth/src/redis/index.js +++ b/packages/auth/src/redis/index.js @@ -21,7 +21,7 @@ function retryConnection() { */ function init() { function errorOccurred(err) { - CONNECTED = false; + CONNECTED = false console.error("Redis connection failed - " + err) setTimeout(() => { init() diff --git a/packages/worker/src/api/routes/admin/users.js b/packages/worker/src/api/routes/admin/users.js index cfff67f3e7..eff873a7b3 100644 --- a/packages/worker/src/api/routes/admin/users.js +++ b/packages/worker/src/api/routes/admin/users.js @@ -7,10 +7,14 @@ const Joi = require("joi") const router = Router() function buildAdminInitValidation() { - return joiValidator.body(Joi.object({ - email: Joi.string().required(), - password: Joi.string().required(), - }).required().unknown(false)) + return joiValidator.body( + Joi.object({ + email: Joi.string().required(), + password: Joi.string().required(), + }) + .required() + .unknown(false) + ) } function buildUserSaveValidation(isSelf = false) { @@ -81,7 +85,11 @@ router buildInviteAcceptValidation(), controller.inviteAccept ) - .post("/api/admin/users/init", buildAdminInitValidation(), controller.adminUser) + .post( + "/api/admin/users/init", + buildAdminInitValidation(), + controller.adminUser + ) .get("/api/admin/users/self", controller.getSelf) // admin endpoint but needs to come at end (blocks other endpoints otherwise) .get("/api/admin/users/:id", adminOnly, controller.find) From 8ce6617e19e366728ecea6ce28e4e367f3219c63 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 14:58:54 +0100 Subject: [PATCH 3/9] Logging and adding better messaging around startup. --- packages/cli/src/hosting/index.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/cli/src/hosting/index.js b/packages/cli/src/hosting/index.js index 60d9f13e80..159f705483 100644 --- a/packages/cli/src/hosting/index.js +++ b/packages/cli/src/hosting/index.js @@ -101,10 +101,11 @@ async function init(type) { async function start() { await checkDockerConfigured() checkInitComplete() - console.log(info("Starting services, this may take a moment.")) + console.log(info("Starting services, this may take a moment - first time this may take a few minutes to download images.")) const port = makeEnv.get("MAIN_PORT") await handleError(async () => { - await compose.upAll({ cwd: "./", log: false }) + // need to log as it makes it more clear + await compose.upAll({ cwd: "./", log: true }) }) console.log( success( From 2abe543cb18c45d574dffe0ce0cc72d96fa3d530 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 16:20:02 +0100 Subject: [PATCH 4/9] Linting and fixing an issue with the dev pass through. --- packages/auth/src/redis/index.js | 5 ----- packages/server/src/api/controllers/dev.js | 9 +++++++++ packages/worker/src/api/controllers/admin/configs.js | 9 +-------- 3 files changed, 10 insertions(+), 13 deletions(-) diff --git a/packages/auth/src/redis/index.js b/packages/auth/src/redis/index.js index 7a1b9a3431..0a21966301 100644 --- a/packages/auth/src/redis/index.js +++ b/packages/auth/src/redis/index.js @@ -4,17 +4,12 @@ const Redis = env.isTest() ? require("ioredis-mock") : require("ioredis") const { addDbPrefix, removeDbPrefix, getRedisOptions } = require("./utils") const RETRY_PERIOD_MS = 2000 -const MAX_RETRIES = 20 const CLUSTERED = false // for testing just generate the client once let CONNECTED = false let CLIENT = env.isTest() ? new Redis(getRedisOptions()) : null -function retryConnection() { - setTimeout(init, RETRY_PERIOD_MS) -} - /** * Inits the system, will error if unable to connect to redis cluster (may take up to 10 seconds) otherwise * will return the ioredis client which will be ready to use. diff --git a/packages/server/src/api/controllers/dev.js b/packages/server/src/api/controllers/dev.js index 2e90fb83e7..068e1e59c0 100644 --- a/packages/server/src/api/controllers/dev.js +++ b/packages/server/src/api/controllers/dev.js @@ -23,7 +23,16 @@ async function redirect(ctx, method) { if (cookie) { ctx.set("set-cookie", cookie) } + let body + try { + body = await response.json() + } catch (err) { + // don't worry about errors, likely no JSON + } ctx.status = response.status + if (body) { + ctx.body = body + } ctx.cookies } diff --git a/packages/worker/src/api/controllers/admin/configs.js b/packages/worker/src/api/controllers/admin/configs.js index 8a6788cdfd..82466249a2 100644 --- a/packages/worker/src/api/controllers/admin/configs.js +++ b/packages/worker/src/api/controllers/admin/configs.js @@ -6,10 +6,8 @@ const { getGlobalUserParams, getScopedFullConfig, } = require("@budibase/auth").db -const fetch = require("node-fetch") const { Configs } = require("../../../constants") const email = require("../../../utilities/email") -const env = require("../../../environment") const { upload, ObjectStoreBuckets } = require("@budibase/auth").objectStore const APP_PREFIX = "app_" @@ -155,12 +153,7 @@ exports.configChecklist = async function (ctx) { // TODO: Watch get started video // Apps exist - let allDbs - if (env.COUCH_DB_URL) { - allDbs = await (await fetch(`${env.COUCH_DB_URL}/_all_dbs`)).json() - } else { - allDbs = await CouchDB.allDbs() - } + let allDbs = await CouchDB.allDbs() const appDbNames = allDbs.filter(dbName => dbName.startsWith(APP_PREFIX)) // They have set up SMTP From 68681fd441ac36fdcc7cd5b24739f9add4a2bcc6 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 16:20:28 +0100 Subject: [PATCH 5/9] Formatting. --- packages/cli/src/hosting/index.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/cli/src/hosting/index.js b/packages/cli/src/hosting/index.js index 159f705483..05d221435c 100644 --- a/packages/cli/src/hosting/index.js +++ b/packages/cli/src/hosting/index.js @@ -101,7 +101,11 @@ async function init(type) { async function start() { await checkDockerConfigured() checkInitComplete() - console.log(info("Starting services, this may take a moment - first time this may take a few minutes to download images.")) + console.log( + info( + "Starting services, this may take a moment - first time this may take a few minutes to download images." + ) + ) const port = makeEnv.get("MAIN_PORT") await handleError(async () => { // need to log as it makes it more clear From ec85f13d5a146535abaa7965c0d2d42f3f0d14e0 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 16:30:24 +0100 Subject: [PATCH 6/9] Adding an initial connection timeout of 5 seconds which after it will retry again. --- packages/auth/src/redis/index.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/packages/auth/src/redis/index.js b/packages/auth/src/redis/index.js index 0a21966301..a3cb6385e6 100644 --- a/packages/auth/src/redis/index.js +++ b/packages/auth/src/redis/index.js @@ -4,6 +4,7 @@ const Redis = env.isTest() ? require("ioredis-mock") : require("ioredis") const { addDbPrefix, removeDbPrefix, getRedisOptions } = require("./utils") const RETRY_PERIOD_MS = 2000 +const STARTUP_TIMEOUT_MS = 5000 const CLUSTERED = false // for testing just generate the client once @@ -15,7 +16,10 @@ let CLIENT = env.isTest() ? new Redis(getRedisOptions()) : null * will return the ioredis client which will be ready to use. */ function init() { + let timeout function errorOccurred(err) { + // always clear this on error + clearTimeout(timeout) CONNECTED = false console.error("Redis connection failed - " + err) setTimeout(() => { @@ -26,6 +30,14 @@ function init() { if (env.isTest() || (CLIENT && CONNECTED)) { return } + // start the timer - only allowed 5 seconds to connect + timeout = setTimeout(() => { + if (!CONNECTED) { + errorOccurred() + } + }, STARTUP_TIMEOUT_MS) + + // disconnect any lingering client if (CLIENT) { CLIENT.disconnect() } @@ -35,6 +47,7 @@ function init() { } else { CLIENT = new Redis(opts) } + // attach handlers CLIENT.on("end", err => { errorOccurred(err) }) @@ -42,6 +55,7 @@ function init() { errorOccurred(err) }) CLIENT.on("connect", () => { + clearTimeout(timeout) CONNECTED = true }) } From bd0f78e38ea93736f5de7d7436ff03d63033f066 Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 16:31:50 +0100 Subject: [PATCH 7/9] Changing how connection is waited for. --- packages/auth/src/redis/index.js | 10 +++++++--- packages/builder/cypress/support/commands.js | 19 +++++++++++++++---- 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/packages/auth/src/redis/index.js b/packages/auth/src/redis/index.js index a3cb6385e6..044a8bcaf9 100644 --- a/packages/auth/src/redis/index.js +++ b/packages/auth/src/redis/index.js @@ -68,9 +68,13 @@ function waitForConnection() { resolve() return } - CLIENT.on("connect", () => { - resolve() - }) + // check if the connection is ready + const interval = setInterval(() => { + if (CONNECTED) { + clearInterval(interval) + resolve() + } + }, 500) }) } diff --git a/packages/builder/cypress/support/commands.js b/packages/builder/cypress/support/commands.js index 4f759a60ea..62365cbe87 100644 --- a/packages/builder/cypress/support/commands.js +++ b/packages/builder/cypress/support/commands.js @@ -11,12 +11,23 @@ Cypress.Commands.add("login", () => { if (cookie) return cy.visit(`localhost:${Cypress.env("PORT")}/builder`) - cy.contains("Create Test User").click() - cy.get("input").first().type("test@test.com") - cy.get('input[type="password"]').type("test") + cy.get("button").then(btn => { + const adminUserButton = "Create super admin user" + console.log(btn.first().first()) + if (!btn.first().contains(adminUserButton)) { + // create admin user + cy.get("input").first().type("test@test.com") + cy.get('input[type="password"]').first().type("test") + cy.get('input[type="password"]').eq(1).type("test") + cy.contains(adminUserButton).click() + } - cy.contains("Login").click() + // login + cy.get("input").first().type("test@test.com") + cy.get('input[type="password"]').type("test") + cy.contains("Login").click() + }) }) }) From 4268ea8eb0551f58fa7161134fdead8ea30d917c Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 16:32:42 +0100 Subject: [PATCH 8/9] Changing cypress commands. --- packages/builder/cypress/support/commands.js | 30 ++++++++++---------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/packages/builder/cypress/support/commands.js b/packages/builder/cypress/support/commands.js index 62365cbe87..80d38937ac 100644 --- a/packages/builder/cypress/support/commands.js +++ b/packages/builder/cypress/support/commands.js @@ -12,22 +12,22 @@ Cypress.Commands.add("login", () => { cy.visit(`localhost:${Cypress.env("PORT")}/builder`) - cy.get("button").then(btn => { - const adminUserButton = "Create super admin user" - console.log(btn.first().first()) - if (!btn.first().contains(adminUserButton)) { - // create admin user - cy.get("input").first().type("test@test.com") - cy.get('input[type="password"]').first().type("test") - cy.get('input[type="password"]').eq(1).type("test") - cy.contains(adminUserButton).click() - } + // cy.get("button").then(btn => { + // const adminUserButton = "Create super admin user" + // console.log(btn.first().first()) + // if (!btn.first().contains(adminUserButton)) { + // // create admin user + // cy.get("input").first().type("test@test.com") + // cy.get('input[type="password"]').first().type("test") + // cy.get('input[type="password"]').eq(1).type("test") + // cy.contains(adminUserButton).click() + // } - // login - cy.get("input").first().type("test@test.com") - cy.get('input[type="password"]').type("test") - cy.contains("Login").click() - }) + // login + cy.get("input").first().type("test@test.com") + cy.get('input[type="password"]').type("test") + cy.contains("Login").click() + // }) }) }) From 8200f2a4e8aa663b0ccd67b1bbaa0c136a9c972a Mon Sep 17 00:00:00 2001 From: mike12345567 Date: Mon, 24 May 2021 17:05:46 +0100 Subject: [PATCH 9/9] Fixing issue with redis updates in tests. --- packages/auth/src/redis/index.js | 36 ++++++++++++++++---------- packages/server/src/app.js | 3 ++- packages/server/src/utilities/redis.js | 5 ++++ 3 files changed, 30 insertions(+), 14 deletions(-) diff --git a/packages/auth/src/redis/index.js b/packages/auth/src/redis/index.js index 044a8bcaf9..7d9e9ad637 100644 --- a/packages/auth/src/redis/index.js +++ b/packages/auth/src/redis/index.js @@ -8,8 +8,24 @@ const STARTUP_TIMEOUT_MS = 5000 const CLUSTERED = false // for testing just generate the client once -let CONNECTED = false +let CLOSED = false let CLIENT = env.isTest() ? new Redis(getRedisOptions()) : null +// if in test always connected +let CONNECTED = !!env.isTest() + +function connectionError(timeout, err) { + // manually shut down, ignore errors + if (CLOSED) { + return + } + // always clear this on error + clearTimeout(timeout) + CONNECTED = false + console.error("Redis connection failed - " + err) + setTimeout(() => { + init() + }, RETRY_PERIOD_MS) +} /** * Inits the system, will error if unable to connect to redis cluster (may take up to 10 seconds) otherwise @@ -17,15 +33,7 @@ let CLIENT = env.isTest() ? new Redis(getRedisOptions()) : null */ function init() { let timeout - function errorOccurred(err) { - // always clear this on error - clearTimeout(timeout) - CONNECTED = false - console.error("Redis connection failed - " + err) - setTimeout(() => { - init() - }, RETRY_PERIOD_MS) - } + CLOSED = false // testing uses a single in memory client if (env.isTest() || (CLIENT && CONNECTED)) { return @@ -33,7 +41,7 @@ function init() { // start the timer - only allowed 5 seconds to connect timeout = setTimeout(() => { if (!CONNECTED) { - errorOccurred() + connectionError(timeout) } }, STARTUP_TIMEOUT_MS) @@ -49,10 +57,10 @@ function init() { } // attach handlers CLIENT.on("end", err => { - errorOccurred(err) + connectionError(timeout, err) }) CLIENT.on("error", err => { - errorOccurred(err) + connectionError(timeout, err) }) CLIENT.on("connect", () => { clearTimeout(timeout) @@ -122,12 +130,14 @@ class RedisWrapper { } async init() { + CLOSED = false init() await waitForConnection() return this } async finish() { + CLOSED = true CLIENT.disconnect() } diff --git a/packages/server/src/app.js b/packages/server/src/app.js index 9ec6c2c687..50df056b2a 100644 --- a/packages/server/src/app.js +++ b/packages/server/src/app.js @@ -73,10 +73,11 @@ if (env.isProd()) { const server = http.createServer(app.callback()) destroyable(server) -server.on("close", () => { +server.on("close", async () => { if (env.NODE_ENV !== "jest") { console.log("Server Closed") } + await redis.shutdown() }) module.exports = server.listen(env.PORT || 0, async () => { diff --git a/packages/server/src/utilities/redis.js b/packages/server/src/utilities/redis.js index 8e0f774f42..ae18b82e02 100644 --- a/packages/server/src/utilities/redis.js +++ b/packages/server/src/utilities/redis.js @@ -11,6 +11,11 @@ exports.init = async () => { debounceClient = await new Client(utils.Databases.DEBOUNCE).init() } +exports.shutdown = async () => { + await devAppClient.finish() + await debounceClient.finish() +} + exports.doesUserHaveLock = async (devAppId, user) => { const value = await devAppClient.get(devAppId) if (!value) {