meta: use load balancer for companion in e2e tests (#4228)

.eslintrc.js

@@ -526,8 +526,8 @@ module.exports = {
       extends: ['plugin:cypress/recommended'],
     },
     {
-      files: ['e2e/**/*.ts', 'e2e/**/*.js', 'e2e/**/*.jsx'],
-      rules: { 'import/no-extraneous-dependencies': 'off', 'no-unused-expressions': 'off' },
+      files: ['e2e/**/*.ts', 'e2e/**/*.js', 'e2e/**/*.jsx', 'e2e/**/*.mjs'],
+      rules: { 'import/no-extraneous-dependencies': 'off', 'no-unused-expressions': 'off', 'no-console': 'off' },
     },
   ],
 }

.github/workflows/e2e.yml

@@ -62,6 +62,12 @@ jobs:
         uses: actions/setup-node@v3
         with:
           node-version: lts/*
+
+      - name: Start Redis
+        uses: supercharge/redis-github-action@1.4.0
+        with:
+          redis-version: 7
+
       - name: Install dependencies
         run: corepack yarn install --immutable
         env:
@@ -72,18 +78,21 @@ jobs:
       - name: Run end-to-end browser tests
         run: corepack yarn run e2e:ci
         env:
+          COMPANION_DATADIR: ./output
+          COMPANION_DOMAIN: localhost:3020
+          COMPANION_PROTOCOL: http
+          COMPANION_REDIS_URL: redis://localhost:6379
           COMPANION_UNSPLASH_KEY: ${{secrets.COMPANION_UNSPLASH_KEY}}
           COMPANION_UNSPLASH_SECRET: ${{secrets.COMPANION_UNSPLASH_SECRET}}
+          COMPANION_AWS_KEY: ${{secrets.COMPANION_AWS_KEY}}
+          COMPANION_AWS_SECRET: ${{secrets.COMPANION_AWS_SECRET}}
+          COMPANION_AWS_BUCKET: ${{secrets.COMPANION_AWS_BUCKET}}
+          COMPANION_AWS_REGION: ${{secrets.COMPANION_AWS_REGION}}
           VITE_COMPANION_URL: http://localhost:3020
           VITE_TRANSLOADIT_KEY: ${{secrets.TRANSLOADIT_KEY}}
           VITE_TRANSLOADIT_SECRET: ${{secrets.TRANSLOADIT_SECRET}}
           VITE_TRANSLOADIT_TEMPLATE: ${{secrets.TRANSLOADIT_TEMPLATE}}
           VITE_TRANSLOADIT_SERVICE_URL: ${{secrets.TRANSLOADIT_SERVICE_URL}}
-          COMPANION_AWS_KEY: ${{secrets.COMPANION_AWS_KEY}}
-          COMPANION_AWS_SECRET: ${{secrets.COMPANION_AWS_SECRET}}
-          COMPANION_AWS_BUCKET: ${{secrets.COMPANION_AWS_BUCKET}}
-          COMPANION_AWS_REGION: ${{secrets.COMPANION_AWS_REGION}}
-          COMPANION_AWS_DISABLE_ACL: 'true'
           # https://docs.cypress.io/guides/references/advanced-installation#Binary-cache
           CYPRESS_CACHE_FOLDER: ${{ steps.cypress-cache-dir-path.outputs.dir }}
       - name: Upload videos in case of failure

e2e/cypress.config.mjs

@@ -1,6 +1,4 @@
-// eslint-disable-next-line import/no-extraneous-dependencies
 import { defineConfig } from 'cypress'
-// eslint-disable-next-line import/no-extraneous-dependencies
 import installLogsPrinter from 'cypress-terminal-report/src/installLogsPrinter.js'
 
 export default defineConfig({
@@ -10,8 +8,7 @@ export default defineConfig({
     baseUrl: 'http://localhost:1234',
     specPattern: 'cypress/integration/*.spec.ts',
 
-    // eslint-disable-next-line no-unused-vars
-    setupNodeEvents (on, config) {
+    setupNodeEvents (on) {
       // implement node event listeners here
       installLogsPrinter(on)
     },

e2e/generate-test.mjs

@@ -1,5 +1,4 @@
 #!/usr/bin/env node
-/* eslint-disable no-console, import/no-extraneous-dependencies */
 import prompts from 'prompts'
 import fs from 'node:fs/promises'
 

e2e/package.json

@@ -48,6 +48,7 @@
     "cypress": "^10.0.0",
     "cypress-terminal-report": "^4.1.2",
     "deep-freeze": "^0.0.1",
+    "execa": "^6.1.0",
     "parcel": "^2.0.1",
     "prompts": "^2.4.2",
     "react": "^18.1.0",

e2e/start-companion-with-load-balancer.mjs

@@ -0,0 +1,79 @@
+#!/usr/bin/env node
+
+import { execa } from 'execa'
+import http from 'node:http'
+import httpProxy from 'http-proxy'
+
+const numInstances = 3
+const lbPort = 3020
+const companionStartPort = 3021
+
+// simple load balancer that will direct requests round robin between companion instances
+function createLoadBalancer (baseUrls) {
+  const proxy = httpProxy.createProxyServer({ ws: true })
+
+  let i = 0
+
+  function getTarget () {
+    return baseUrls[i % baseUrls.length]
+  }
+
+  const server = http.createServer((req, res) => {
+    const target = getTarget()
+    // console.log('req', req.method, target, req.url)
+    proxy.web(req, res, { target }, (err) => {
+      console.error('Load balancer failed to proxy request', err.message)
+      res.statusCode = 500
+      res.end()
+    })
+    i++
+  })
+
+  server.on('upgrade', (req, socket, head) => {
+    const target = getTarget()
+    // console.log('upgrade', target, req.url)
+    proxy.ws(req, socket, head, { target }, (err) => {
+      console.error('Load balancer failed to proxy websocket', err.message)
+      console.error(err)
+      socket.destroy()
+    })
+    i++
+  })
+
+  server.listen(lbPort)
+  console.log('Load balancer listening', lbPort)
+  return server
+}
+
+const startCompanion = ({ name, port }) => execa('nodemon', [
+  '--watch', 'packages/@uppy/companion/src', '--exec', 'node', '-r', 'dotenv/config', './packages/@uppy/companion/src/standalone/start-server.js',
+], {
+  cwd: new URL('../', import.meta.url),
+  stdio: 'inherit',
+  env: {
+    // Note: these env variables will override anything set in .env
+    COMPANION_PORT: port,
+    COMPANION_SECRET: 'development', // multi instance will not work without secret set
+    COMPANION_PREAUTH_SECRET: 'development', // multi instance will not work without secret set
+    COMPANION_ALLOW_LOCAL_URLS: 'true',
+    COMPANION_LOGGER_PROCESS_NAME: name,
+  },
+})
+
+const hosts = Array.from({ length: numInstances }, (_, index) => {
+  const port = companionStartPort + index
+  return { index, port }
+})
+
+console.log('Starting companion instances on ports', hosts.map(({ port }) => port))
+
+const companions = hosts.map(({ index, port }) => startCompanion({ name: `companion${index}`, port }))
+
+let loadBalancer
+try {
+  loadBalancer = createLoadBalancer(hosts.map(({ port }) => `http://localhost:${port}`))
+  await Promise.all(companions)
+} finally {
+  loadBalancer?.close()
+  companions.forEach((companion) => companion.kill())
+}

package.json

@@ -130,10 +130,11 @@
     "release": "PACKAGES=$(yarn workspaces list --json) yarn workspace @uppy-dev/release interactive",
     "size": "echo 'JS Bundle mingz:' && cat ./packages/uppy/dist/uppy.min.js | gzip | wc -c && echo 'CSS Bundle mingz:' && cat ./packages/uppy/dist/uppy.min.css | gzip | wc -c",
     "start:companion": "bash bin/companion.sh",
+    "start:companion:with-loadbalancer": "e2e/start-companion-with-load-balancer.mjs",
     "start": "npm-run-all --parallel watch start:companion web:start",
     "e2e": "yarn build && yarn e2e:skip-build",
-    "e2e:skip-build": "npm-run-all --parallel watch:js:lib e2e:client start:companion e2e:cypress",
-    "e2e:ci": "start-server-and-test 'npm-run-all --parallel e2e:client start:companion' '1234|3020' e2e:headless",
+    "e2e:skip-build": "npm-run-all --parallel watch:js:lib e2e:client start:companion:with-loadbalancer e2e:cypress",
+    "e2e:ci": "start-server-and-test 'npm-run-all --parallel e2e:client start:companion:with-loadbalancer' '1234|3020' e2e:headless",
     "e2e:client": "yarn workspace e2e client:start",
     "e2e:cypress": "yarn workspace e2e cypress:open",
     "e2e:headless": "yarn workspace e2e cypress:headless",

packages/@uppy/companion/src/companion.js

@@ -21,6 +21,10 @@ const { getCredentialsOverrideMiddleware } = require('./server/provider/credenti
 // @ts-ignore
 const { version } = require('../package.json')
 
+function setLoggerProcessName ({ loggerProcessName }) {
+  if (loggerProcessName != null) logger.setProcessName(loggerProcessName)
+}
+
 // intercepts grantJS' default response error when something goes
 // wrong during oauth process.
 const interceptGrantErrorResponse = interceptor((req, res) => {
@@ -51,6 +55,8 @@ const interceptGrantErrorResponse = interceptor((req, res) => {
 module.exports.errors = { ProviderApiError, ProviderAuthError }
 module.exports.socket = require('./server/socket')
 
+module.exports.setLoggerProcessName = setLoggerProcessName
+
 /**
  * Entry point into initializing the Companion app.
  *
@@ -58,6 +64,8 @@ module.exports.socket = require('./server/socket')
  * @returns {{ app: import('express').Express, emitter: any }}}
  */
 module.exports.app = (optionsArg = {}) => {
+  setLoggerProcessName(optionsArg)
+
   validateConfig(optionsArg)
 
   const options = merge({}, defaultOptions, optionsArg)

packages/@uppy/companion/src/server/logger.js

@@ -33,18 +33,25 @@ function maskMessage (msg) {
   return out
 }
 
+let processName = 'companion'
+
+exports.setProcessName = (newProcessName) => {
+  processName = newProcessName
+}
+
 /**
  * message log
  *
- * @param {string | Error} arg the message or error to log
- * @param {string} tag a unique tag to easily search for this message
- * @param {string} level error | info | debug
- * @param {string} [id] a unique id to easily trace logs tied to a request
- * @param {Function} [color] function to display the log in appropriate color
+ * @param {object} params
+ * @param {string | Error} params.arg the message or error to log
+ * @param {string} params.tag a unique tag to easily search for this message
+ * @param {string} params.level error | info | debug
+ * @param {string} [params.traceId] a unique id to easily trace logs tied to a request
+ * @param {Function} [params.color] function to display the log in appropriate color
  */
-const log = (arg, tag = '', level, id = '', color = (message) => message) => {
+const log = ({ arg, tag = '', level, traceId = '', color = (message) => message }) => {
   const time = new Date().toISOString()
-  const whitespace = tag && id ? ' ' : ''
+  const whitespace = tag && traceId ? ' ' : ''
 
   function msgToString () {
     // We don't need to log stack trace on special errors that we ourselves have produced
@@ -59,7 +66,7 @@ const log = (arg, tag = '', level, id = '', color = (message) => message) => {
   const msgString = msgToString()
   const masked = maskMessage(msgString)
   // eslint-disable-next-line no-console
-  console.log(color(`companion: ${time} [${level}] ${id}${whitespace}${tag}`), color(masked))
+  console.log(color(`${processName}: ${time} [${level}] ${traceId}${whitespace}${tag}`), color(masked))
 }
 
 /**
@@ -70,7 +77,7 @@ const log = (arg, tag = '', level, id = '', color = (message) => message) => {
  * @param {string} [traceId] a unique id to easily trace logs tied to a request
  */
 exports.info = (msg, tag, traceId) => {
-  log(msg, tag, 'info', traceId)
+  log({ arg: msg, tag, level: 'info', traceId })
 }
 
 /**
@@ -81,8 +88,7 @@ exports.info = (msg, tag, traceId) => {
  * @param {string} [traceId] a unique id to easily trace logs tied to a request
  */
 exports.warn = (msg, tag, traceId) => {
-  // @ts-ignore
-  log(msg, tag, 'warn', traceId, chalk.bold.yellow)
+  log({ arg: msg, tag, level: 'warn', traceId, color: chalk.bold.yellow })
 }
 
 /**
@@ -93,8 +99,7 @@ exports.warn = (msg, tag, traceId) => {
  * @param {string} [traceId] a unique id to easily trace logs tied to a request
  */
 exports.error = (msg, tag, traceId) => {
-  // @ts-ignore
-  log(msg, tag, 'error', traceId, chalk.bold.red)
+  log({ arg: msg, tag, level: 'error', traceId, color: chalk.bold.red })
 }
 
 /**
@@ -106,7 +111,6 @@ exports.error = (msg, tag, traceId) => {
  */
 exports.debug = (msg, tag, traceId) => {
   if (process.env.NODE_ENV !== 'production') {
-    // @ts-ignore
-    log(msg, tag, 'debug', traceId, chalk.bold.blue)
+    log({ arg: msg, tag, level: 'debug', traceId, color: chalk.bold.blue })
   }
 }

packages/@uppy/companion/src/standalone/helper.js

@@ -9,13 +9,48 @@ const logger = require('../server/logger')
 const { version } = require('../../package.json')
 
 /**
- * Reads all companion configuration set via environment variables
- * and via the config file path
+ * Tries to read the secret from a file if the according environment variable is set.
+ * Otherwise it falls back to the standard secret environment variable.
  *
- * @returns {object}
+ * @param {string} baseEnvVar
+ *
+ * @returns {string}
  */
-exports.getCompanionOptions = (options = {}) => {
-  return merge({}, getConfigFromEnv(), getConfigFromFile(), options)
+const getSecret = (baseEnvVar) => {
+  const secretFile = process.env[`${baseEnvVar}_FILE`]
+  return secretFile
+    ? fs.readFileSync(secretFile).toString()
+    : process.env[baseEnvVar]
+}
+
+/**
+ * Auto-generates server secret
+ *
+ * @returns {string}
+ */
+exports.generateSecret = () => {
+  logger.warn('auto-generating server secret because none was specified', 'startup.secret')
+  return crypto.randomBytes(64).toString('hex')
+}
+
+/**
+ *
+ * @param {string} url
+ */
+const hasProtocol = (url) => {
+  return url.startsWith('https://') || url.startsWith('http://')
+}
+
+function getCorsOrigins () {
+  if (process.env.COMPANION_CLIENT_ORIGINS) {
+    return process.env.COMPANION_CLIENT_ORIGINS
+      .split(',')
+      .map((url) => (hasProtocol(url) ? url : `${process.env.COMPANION_PROTOCOL || 'http'}://${url}`))
+  }
+  if (process.env.COMPANION_CLIENT_ORIGINS_REGEX) {
+    return new RegExp(process.env.COMPANION_CLIENT_ORIGINS_REGEX)
+  }
+  return undefined
 }
 
 /**
@@ -104,8 +139,8 @@ const getConfigFromEnv = () => {
     redisOptions: {},
     sendSelfEndpoint: process.env.COMPANION_SELF_ENDPOINT,
     uploadUrls: uploadUrls ? uploadUrls.split(',') : null,
-    secret: getSecret('COMPANION_SECRET') || generateSecret(),
-    preAuthSecret: getSecret('COMPANION_PREAUTH_SECRET') || generateSecret(),
+    secret: getSecret('COMPANION_SECRET'),
+    preAuthSecret: getSecret('COMPANION_PREAUTH_SECRET'),
     allowLocalUrls: process.env.COMPANION_ALLOW_LOCAL_URLS === 'true',
     // cookieDomain is kind of a hack to support distributed systems. This should be improved but we never got so far.
     cookieDomain: process.env.COMPANION_COOKIE_DOMAIN,
@@ -115,32 +150,29 @@ const getConfigFromEnv = () => {
     clientSocketConnectTimeout: process.env.COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT
       ? parseInt(process.env.COMPANION_CLIENT_SOCKET_CONNECT_TIMEOUT, 10) : undefined,
     metrics: process.env.COMPANION_HIDE_METRICS !== 'true',
+    loggerProcessName: process.env.COMPANION_LOGGER_PROCESS_NAME,
+    corsOrigins: getCorsOrigins(),
   }
 }
 
 /**
- * Tries to read the secret from a file if the according environment variable is set.
- * Otherwise it falls back to the standard secret environment variable.
- *
- * @param {string} baseEnvVar
+ * Returns the config path specified via cli arguments
  *
  * @returns {string}
  */
-const getSecret = (baseEnvVar) => {
-  const secretFile = process.env[`${baseEnvVar}_FILE`]
-  return secretFile
-    ? fs.readFileSync(secretFile).toString()
-    : process.env[baseEnvVar]
-}
+const getConfigPath = () => {
+  let configPath
 
-/**
- * Auto-generates server secret
- *
- * @returns {string}
- */
-const generateSecret = () => {
-  logger.warn('auto-generating server secret because none was specified', 'startup.secret')
-  return crypto.randomBytes(64).toString('hex')
+  for (let i = process.argv.length - 1; i >= 0; i--) {
+    const isConfigFlag = process.argv[i] === '-c' || process.argv[i] === '--config'
+    const flagHasValue = i + 1 <= process.argv.length
+    if (isConfigFlag && flagHasValue) {
+      configPath = process.argv[i + 1]
+      break
+    }
+  }
+
+  return configPath
 }
 
 /**
@@ -158,31 +190,13 @@ const getConfigFromFile = () => {
 }
 
 /**
- * Returns the config path specified via cli arguments
- *
- * @returns {string}
- */
-const getConfigPath = () => {
-  let configPath
-
-  for (let i = process.argv.length - 1; i >= 0; i--) {
-    const isConfigFlag = process.argv[i] === '-c' || process.argv[i] === '--config'
-    const flagHasValue = i + 1 <= process.argv.length
-    if (isConfigFlag && flagHasValue) {
-      configPath = process.argv[i + 1]
-      break
-    }
-  }
-
-  return configPath
-}
-
-/**
+ * Reads all companion configuration set via environment variables
+ * and via the config file path
  *
- * @param {string} url
+ * @returns {object}
  */
-exports.hasProtocol = (url) => {
-  return url.startsWith('http://') || url.startsWith('https://')
+exports.getCompanionOptions = (options = {}) => {
+  return merge({}, getConfigFromEnv(), getConfigFromFile(), options)
 }
 
 exports.buildHelpfulStartupMessage = (companionOptions) => {

packages/@uppy/companion/src/standalone/index.js

@@ -11,25 +11,20 @@ const connectRedis = require('connect-redis')
 const logger = require('../server/logger')
 const redis = require('../server/redis')
 const companion = require('../companion')
-const helper = require('./helper')
+const { getCompanionOptions, generateSecret, buildHelpfulStartupMessage } = require('./helper')
 
 /**
  * Configures an Express app for running Companion standalone
  *
  * @returns {object}
  */
-module.exports = function server (inputCompanionOptions = {}) {
-  let corsOrigins
-  if (process.env.COMPANION_CLIENT_ORIGINS) {
-    corsOrigins = process.env.COMPANION_CLIENT_ORIGINS
-      .split(',')
-      .map((url) => (helper.hasProtocol(url) ? url : `${process.env.COMPANION_PROTOCOL || 'http'}://${url}`))
-  } else if (process.env.COMPANION_CLIENT_ORIGINS_REGEX) {
-    corsOrigins = new RegExp(process.env.COMPANION_CLIENT_ORIGINS_REGEX)
-  }
+module.exports = function server (inputCompanionOptions) {
+  const companionOptions = getCompanionOptions(inputCompanionOptions)
+
+  companion.setLoggerProcessName(companionOptions)
 
-  const moreCompanionOptions = { ...inputCompanionOptions, corsOrigins }
-  const companionOptions = helper.getCompanionOptions(moreCompanionOptions)
+  if (!companionOptions.secret) companionOptions.secret = generateSecret()
+  if (!companionOptions.preAuthSecret) companionOptions.preAuthSecret = generateSecret()
 
   const app = express()
 
@@ -98,6 +93,7 @@ module.exports = function server (inputCompanionOptions = {}) {
       const { query, censored } = censorQuery(rawQuery)
       return censored ? `${parsed.href.split('?')[0]}?${qs.stringify(query)}` : parsed.href
     }
+    return undefined
   })
 
   router.use(bodyParser.json())
@@ -141,7 +137,7 @@ module.exports = function server (inputCompanionOptions = {}) {
   if (process.env.COMPANION_HIDE_WELCOME !== 'true') {
     router.get('/', (req, res) => {
       res.setHeader('Content-Type', 'text/plain')
-      res.send(helper.buildHelpfulStartupMessage(companionOptions))
+      res.send(buildHelpfulStartupMessage(companionOptions))
     })
   }
 

packages/@uppy/companion/src/standalone/start-server.js

@@ -3,16 +3,13 @@ const companion = require('../companion')
 // @ts-ignore
 const { version } = require('../../package.json')
 const standalone = require('.')
-const { getURLBuilder } = require('../server/helpers/utils')
+const logger = require('../server/logger')
 
 const port = process.env.COMPANION_PORT || process.env.PORT || 3020
 
-const { app, companionOptions } = standalone()
+const { app } = standalone()
 
 companion.socket(app.listen(port))
 
-const buildURL = getURLBuilder(companionOptions)
-
-/* eslint-disable no-console */
-console.log(`Welcome to Companion! v${version}`)
-console.log(`Listening on ${buildURL('/', false)}`)
+logger.info(`Welcome to Companion! v${version}`)
+logger.info(`Listening on http://localhost:${port}`)

website/src/docs/companion.md

@@ -191,6 +191,8 @@ export COMPANION_PATH="/SERVER/PATH/TO/WHERE/COMPANION/LIVES"
 export COMPANION_HIDE_WELCOME="true"
 # disables the metrics page, defaults to false
 export COMPANION_HIDE_METRICS="true"
+# prefix all log entries with this value - useful for multiple instances
+export COMPANION_LOGGER_PROCESS_NAME="companion"
 
 # use this in place of COMPANION_PATH if the server path should not be
 # handled by the express.js app, but maybe by an external server configuration

yarn.lock

@@ -15584,6 +15584,7 @@ __metadata:
     cypress: ^10.0.0
     cypress-terminal-report: ^4.1.2
     deep-freeze: ^0.0.1
+    execa: ^6.1.0
     parcel: ^2.0.1
     prompts: ^2.4.2
     react: ^18.1.0