From 9169114e607f3be465693b10ebf6304c65e349da Mon Sep 17 00:00:00 2001 From: Daniel Salazar Date: Fri, 13 Mar 2026 13:41:07 -0700 Subject: [PATCH] fix: tests oom breaking (#2661) * fix: healthcheck route visible in subdomains * fix: tests oom breaking * fix: don't spawn many dynalites * fix: possible loop when merging self owned apps --- .github/workflows/test.yml | 8 +-- src/backend/src/clients/dynamodb/DDBClient.ts | 65 ++++++++++++++----- .../src/modules/data-access/AppService.js | 26 +++++++- .../modules/data-access/AppService.test.js | 11 +++- .../src/modules/web/WebServerService.js | 3 +- src/backend/src/om/entitystorage/AppES.js | 39 ++++++++++- src/backend/src/routers/healthcheck.js | 49 +++++++++++++- src/backend/vitest.config.ts | 3 - 8 files changed, 173 insertions(+), 31 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 86aedf5a6..b657c8bba 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -28,13 +28,11 @@ jobs: - name: Backend Tests (with coverage) env: - NODE_OPTIONS: --max-old-space-size=8192 + NODE_OPTIONS: --max-old-space-size=4096 run: | - rm package-lock.json - npm install -g npm@latest - npm install + npm ci npm run build - npm run test:backend -- --coverage --maxWorkers=2 --coverage.reporter=json --coverage.reporter=json-summary --coverage.reporter=lcov + npm run test:backend -- --coverage --coverage.reporter=json --coverage.reporter=json-summary --coverage.reporter=lcov - name: Upload backend coverage report if: ${{ always() && hashFiles('coverage/**/coverage-summary.json') != '' }} diff --git a/src/backend/src/clients/dynamodb/DDBClient.ts b/src/backend/src/clients/dynamodb/DDBClient.ts index eb0374ec9..f40f98c91 100644 --- a/src/backend/src/clients/dynamodb/DDBClient.ts +++ b/src/backend/src/clients/dynamodb/DDBClient.ts @@ -15,6 +15,43 @@ interface DBClientConfig { endpoint?: string } +const LOCAL_DYNAMO_PATH_KEY = ':memory:'; +const localDynaliteEndpointPromises = new Map>(); + +const getDynalitePathKey = (path?: string) => { + if ( path === ':memory:' ) return LOCAL_DYNAMO_PATH_KEY; + return path || './puter-ddb'; +}; + +const getOrCreateLocalDynaliteEndpoint = async (pathKey: string) => { + let endpointPromise = localDynaliteEndpointPromises.get(pathKey); + if ( endpointPromise ) return endpointPromise; + + endpointPromise = (async () => { + const dynaliteOptions = pathKey === LOCAL_DYNAMO_PATH_KEY + ? { createTableMs: 0 } + : { createTableMs: 0, path: pathKey }; + + const dynaliteInstance = dynalite(dynaliteOptions); + const dynaliteServer = dynaliteInstance.listen(0, '127.0.0.1'); + // Don't keep test workers alive just because dynalite is still open. + dynaliteServer.unref?.(); + await once(dynaliteServer, 'listening'); + + const address = dynaliteServer.address(); + const port = (typeof address === 'object' && address ? address.port : undefined) || 4567; + return `http://127.0.0.1:${port}`; + })(); + + localDynaliteEndpointPromises.set(pathKey, endpointPromise); + endpointPromise.catch(() => { + if ( localDynaliteEndpointPromises.get(pathKey) === endpointPromise ) { + localDynaliteEndpointPromises.delete(pathKey); + } + }); + return endpointPromise; +}; + export class DDBClient { ddbClientPromise: Promise; #documentClient!: DynamoDBDocumentClient; @@ -42,12 +79,8 @@ export class DDBClient { async #getClient () { if ( ! this.config?.aws ) { console.warn('No config for DynamoDB, will fall back on local dynalite'); - const dynaliteInstance = dynalite({ createTableMs: 0, path: this.config?.path === ':memory:' ? undefined : this.config?.path || './puter-ddb' }); - const dynaliteServer = dynaliteInstance.listen(0, '127.0.0.1'); - await once(dynaliteServer, 'listening'); - const address = dynaliteServer.address(); - const port = (typeof address === 'object' && address ? address.port : undefined) || 4567; - const dynamoEndpoint = `http://127.0.0.1:${port}`; + const pathKey = getDynalitePathKey(this.config?.path); + const dynamoEndpoint = await getOrCreateLocalDynaliteEndpoint(pathKey); const client = new DynamoDBClient({ credentials: { @@ -117,15 +150,17 @@ export class DDBClient { return acc; }, {} as Record[]>); - const RequestItems: BatchGetCommandInput['RequestItems'] = Object.entries(allRequestItemsPerTable).reduce((acc, [table, keyList]) => { - const Keys = keyList; - acc[table] = { - Keys, - ConsistentRead: consistentRead, - }; - return acc; - }, - {} as NonNullable); + const RequestItems: BatchGetCommandInput['RequestItems'] = Object.entries(allRequestItemsPerTable).reduce( + (acc, [table, keyList]) => { + const Keys = keyList; + acc[table] = { + Keys, + ConsistentRead: consistentRead, + }; + return acc; + }, + {} as NonNullable, + ); const command = new BatchGetCommand({ RequestItems, diff --git a/src/backend/src/modules/data-access/AppService.js b/src/backend/src/modules/data-access/AppService.js index ee0ac5c51..842c84ac2 100644 --- a/src/backend/src/modules/data-access/AppService.js +++ b/src/backend/src/modules/data-access/AppService.js @@ -1025,6 +1025,7 @@ export default class AppService extends BaseService { // Handle app-specific logic (AppES behavior) const user = actor.type.user; + const oldAppId = await this.#resolveAppId(old_app); // Ensure puter.site subdomain is owned by user (if index_url changed) if ( object.index_url && object.index_url !== old_app.index_url ) { @@ -1033,14 +1034,14 @@ export default class AppService extends BaseService { object, options, user, - excludeAppId: old_app.id, + excludeAppId: oldAppId, }); if ( joinedApp ) { return joinedApp; } await this.#ensureIndexUrlNotAlreadyInUse({ indexUrl: object.index_url, - excludeAppId: old_app.id, + excludeAppId: oldAppId, }); } @@ -1065,6 +1066,20 @@ export default class AppService extends BaseService { return await this.#read({ uid: old_app.uid }); } + async #resolveAppId (app) { + const appId = Number(app?.id); + if ( Number.isInteger(appId) && appId > 0 ) return appId; + if ( typeof app?.uid !== 'string' || !app.uid ) return undefined; + + const rows = await this.db.read( + 'SELECT id FROM apps WHERE uid = ? LIMIT 1', + [app.uid], + ); + const resolvedId = Number(rows?.[0]?.id); + if ( Number.isInteger(resolvedId) && resolvedId > 0 ) return resolvedId; + return undefined; + } + async #check_owner_permission (old_app) { const svc_permission = this.services.get('permission'); const actor = Context.get('actor'); @@ -1244,6 +1259,13 @@ export default class AppService extends BaseService { const rows = await this.db.read(query, parameters); const conflictRow = rows.find(row => { + if ( + Number.isInteger(excludeAppId) + && excludeAppId > 0 + && Number(row?.id) === excludeAppId + ) { + return false; + } if ( typeof row?.index_url === 'string' ) { return indexUrlCandidates.includes(row.index_url); } diff --git a/src/backend/src/modules/data-access/AppService.test.js b/src/backend/src/modules/data-access/AppService.test.js index 5fa6525f0..7e0dc6816 100644 --- a/src/backend/src/modules/data-access/AppService.test.js +++ b/src/backend/src/modules/data-access/AppService.test.js @@ -1837,8 +1837,17 @@ describe('AppService', () => { it('should join existing unowned hosted app when index_url is already in use on update', async () => { setupContextForWrite(createMockUserActor(1)); mockPuterSiteService.get_subdomain.mockResolvedValue({ user_id: 1 }); + let readCallCount = 0; mockDb.read.mockImplementation(async (query, params) => { + readCallCount++; + if ( readCallCount > 100 ) { + throw new Error(`excessive mockDb.read calls in join test: ${String(query)} :: ${JSON.stringify(params)}`); + } if ( typeof query === 'string' && query.includes('FROM apps WHERE index_url IN') ) { + if ( Array.isArray(params) && params[params.length - 1] === 777 ) { + // Mirrors SQL `AND id != ?` behavior during join follow-up updates. + return []; + } return [{ id: 777, uid: 'app-conflict-uid', @@ -1878,7 +1887,7 @@ describe('AppService', () => { expect(result.uid).toBe('app-conflict-uid'); expect(mockDbWrite.write).toHaveBeenCalledWith( expect.stringContaining('UPDATE apps SET'), - expect.arrayContaining(['Joined Update Title', 777]), + expect.arrayContaining(['Joined Update Title', 'app-conflict-uid']), ); expect(mockAppInformationService.delete_app).toHaveBeenCalledWith( 'app-uid-123', diff --git a/src/backend/src/modules/web/WebServerService.js b/src/backend/src/modules/web/WebServerService.js index c0bc3a05a..70769efb1 100644 --- a/src/backend/src/modules/web/WebServerService.js +++ b/src/backend/src/modules/web/WebServerService.js @@ -540,8 +540,7 @@ class WebServerService extends BaseService { // Check if the hostname matches any of the allowed domains or is a subdomain of an allowed domain // Exception: allow /healthcheck endpoint on the root domain if ( - req.path === '/healthcheck' && - hostName === config.domain.toLowerCase() + req.path === '/healthcheck' ) { next(); return; diff --git a/src/backend/src/om/entitystorage/AppES.js b/src/backend/src/om/entitystorage/AppES.js index 679f5807e..4fa125bb1 100644 --- a/src/backend/src/om/entitystorage/AppES.js +++ b/src/backend/src/om/entitystorage/AppES.js @@ -679,7 +679,15 @@ class AppES extends BaseES { query += ' ORDER BY timestamp ASC, id ASC LIMIT 1'; const rows = await this.db.read(query, parameters); + const normalizedExcludeMysqlId = Number(excludeMysqlId); const conflictRow = rows.find(row => { + if ( + Number.isInteger(normalizedExcludeMysqlId) + && normalizedExcludeMysqlId > 0 + && Number(row?.id) === normalizedExcludeMysqlId + ) { + return false; + } if ( typeof row?.index_url === 'string' ) { return candidates.includes(row.index_url); } @@ -688,6 +696,33 @@ class AppES extends BaseES { return conflictRow || null; }, + async resolve_entity_mysql_id_ (entity) { + const directMysqlId = Number(entity?.private_meta?.mysql_id); + if ( Number.isInteger(directMysqlId) && directMysqlId > 0 ) { + return directMysqlId; + } + + if ( !entity || typeof entity.get !== 'function' ) { + return undefined; + } + + const uid = await entity.get('uid'); + if ( typeof uid !== 'string' || !uid ) { + return undefined; + } + + const rows = await this.db.read( + 'SELECT id FROM apps WHERE uid = ? LIMIT 1', + [uid], + ); + const mysqlId = Number(rows?.[0]?.id); + if ( Number.isInteger(mysqlId) && mysqlId > 0 ) { + return mysqlId; + } + + return undefined; + }, + async claim_app_ownership_by_id_for_user_ ({ appId, userId }) { if ( !Number.isInteger(appId) || appId <= 0 ) return; if ( !Number.isInteger(userId) || userId <= 0 ) return; @@ -783,7 +818,7 @@ class AppES extends BaseES { const new_index_url = await entity.get('index_url'); const source_entity = extra.old_entity; - const currentMysqlId = extra.old_entity?.private_meta?.mysql_id; + const currentMysqlId = await this.resolve_entity_mysql_id_(extra.old_entity); const conflictRow = await this.find_index_url_conflict_({ indexUrl: new_index_url, excludeMysqlId: currentMysqlId, @@ -918,7 +953,7 @@ class AppES extends BaseES { } } - const currentMysqlId = extra.old_entity?.private_meta?.mysql_id; + const currentMysqlId = await this.resolve_entity_mysql_id_(extra.old_entity); const conflictRow = await this.find_index_url_conflict_({ indexUrl: new_index_url, excludeMysqlId: currentMysqlId, diff --git a/src/backend/src/routers/healthcheck.js b/src/backend/src/routers/healthcheck.js index 36234458a..fd64b15d1 100644 --- a/src/backend/src/routers/healthcheck.js +++ b/src/backend/src/routers/healthcheck.js @@ -18,12 +18,59 @@ */ 'use strict'; const express = require('express'); +const config = require('../config'); const router = new express.Router(); +const normalizeHostDomain = (domain) => { + if ( typeof domain !== 'string' ) return null; + const normalizedDomain = domain.trim().toLowerCase().replace(/^\./, ''); + if ( ! normalizedDomain ) return null; + + try { + return new URL(`http://${normalizedDomain}`).hostname.toLowerCase(); + } catch { + return normalizedDomain.split(':')[0] || null; + } +}; + +const hostMatchesDomain = (hostname, domain) => { + const normalizedHost = normalizeHostDomain(hostname); + const normalizedDomain = normalizeHostDomain(domain); + if ( !normalizedHost || !normalizedDomain ) return false; + return normalizedHost === normalizedDomain || + normalizedHost.endsWith(`.${normalizedDomain}`); +}; + +const isHostedDomainRequest = (req) => { + const requestHost = normalizeHostDomain(req.hostname ?? req.headers?.host); + if ( ! requestHost ) return false; + + const hostedDomains = new Set(); + for ( const domain of [ + config.static_hosting_domain, + config.static_hosting_domain_alt, + config.private_app_hosting_domain, + config.private_app_hosting_domain_alt, + ] ) { + const normalizedDomain = normalizeHostDomain(domain); + if ( normalizedDomain ) { + hostedDomains.add(normalizedDomain); + } + } + + return [...hostedDomains].some(hostedDomain => + hostMatchesDomain(requestHost, hostedDomain)); +}; + // -----------------------------------------------------------------------// // GET /healthcheck // -----------------------------------------------------------------------// -router.get('/healthcheck', async (req, res) => { +router.get('/healthcheck', async (req, res, next) => { + if ( isHostedDomainRequest(req) ) { + next(); + return; + } + const svc_serverHealth = req.services.get('server-health'); const status = await svc_serverHealth.get_status(); diff --git a/src/backend/vitest.config.ts b/src/backend/vitest.config.ts index ce510d122..a27d1c7cc 100644 --- a/src/backend/vitest.config.ts +++ b/src/backend/vitest.config.ts @@ -7,14 +7,11 @@ const isCi = process.env.CI === 'true'; export default defineConfig(({ mode }) => ({ test: { globals: true, - maxWorkers: isCi ? 2 : undefined, - minWorkers: isCi ? 1 : undefined, coverage: { provider: 'v8', reporter: isCi ? ['json', 'json-summary', 'lcov'] : ['text', 'json', 'json-summary', 'html', 'lcov'], - processingConcurrency: isCi ? 2 : undefined, excludeAfterRemap: true, // Keep coverage focused on executed files to avoid high-memory // uncovered-file remapping in CI.