From 9530a5f914d6159e6533ca3395b9076d9d7b7d2e Mon Sep 17 00:00:00 2001 From: fflorent Date: Tue, 5 Nov 2024 11:17:14 +0100 Subject: [PATCH] Attempt to de-asyncify serverUrl computation --- test/server/lib/DocApi.ts | 34 ++++++------ test/server/lib/ManyFetches.ts | 8 +-- test/server/lib/UnhandledErrors.ts | 4 +- test/server/lib/Webhooks-Proxy.ts | 11 ++-- test/server/lib/helpers/TestServer.ts | 75 +++++++++++---------------- 5 files changed, 60 insertions(+), 72 deletions(-) diff --git a/test/server/lib/DocApi.ts b/test/server/lib/DocApi.ts index fca4c48f55..c9edcbe243 100644 --- a/test/server/lib/DocApi.ts +++ b/test/server/lib/DocApi.ts @@ -15,7 +15,7 @@ import { getDocApiUsageKeysToIncr, WebhookSubscription } from 'app/server/lib/DocApi'; -import {delayAbort} from 'app/server/lib/serverUtils'; +import {delayAbort, getAvailablePort} from 'app/server/lib/serverUtils'; import axios, {AxiosRequestConfig, AxiosResponse} from 'axios'; import {delay} from 'bluebird'; import {assert} from 'chai'; @@ -141,7 +141,7 @@ describe('DocApi', function () { GRIST_DATA_DIR: dataDir }; home = docs = await TestServer.startServer('home,docs', tmpDir, suitename, additionalEnvConfiguration); - homeUrl = serverUrl = await home.getServerUrl(); + homeUrl = serverUrl = home.serverUrl; hasHomeApi = true; }); testDocApi({webhooksTestPort}); @@ -155,7 +155,7 @@ describe('DocApi', function () { GRIST_ANON_PLAYGROUND: 'false' }; home = docs = await TestServer.startServer('home,docs', tmpDir, suitename, additionalEnvConfiguration); - homeUrl = serverUrl = await home.getServerUrl(); + homeUrl = serverUrl = home.serverUrl; hasHomeApi = true; }); @@ -176,7 +176,7 @@ describe('DocApi', function () { }; home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); - homeUrl = serverUrl = await home.getServerUrl(); + homeUrl = serverUrl = home.serverUrl; docs = await TestServer.startServer('docs', tmpDir, suitename, additionalEnvConfiguration, homeUrl); hasHomeApi = true; }); @@ -185,30 +185,32 @@ describe('DocApi', function () { describe('behind a reverse-proxy', function () { async function setupServersWithProxy(suitename: string, overrideEnvConf?: NodeJS.ProcessEnv) { - const proxy = new TestServerReverseProxy(); + const proxy = await TestServerReverseProxy.build(); const additionalEnvConfiguration = { ALLOWED_WEBHOOK_DOMAINS: `example.com,localhost:${webhooksTestPort}`, GRIST_DATA_DIR: dataDir, - APP_HOME_URL: await proxy.getServerUrl(), + APP_HOME_URL: proxy.serverUrl, GRIST_ORG_IN_PATH: 'true', GRIST_SINGLE_PORT: '0', ...overrideEnvConf }; - const home = new TestServer('home', tmpDir, suitename); - await home.start(await home.getServerUrl(), additionalEnvConfiguration); - const docs = new TestServer('docs', tmpDir, suitename); - await docs.start(await home.getServerUrl(), { + const homePort = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + const home = new TestServer('home', homePort, tmpDir, suitename); + await home.start(home.serverUrl, additionalEnvConfiguration); + const docPort = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + const docs = new TestServer('docs', docPort, tmpDir, suitename); + await docs.start(home.serverUrl, { ...additionalEnvConfiguration, - APP_DOC_URL: `${await proxy.getServerUrl()}/dw/dw1`, - APP_DOC_INTERNAL_URL: await docs.getServerUrl(), + APP_DOC_URL: `${proxy.serverUrl}/dw/dw1`, + APP_DOC_INTERNAL_URL: docs.serverUrl, }); proxy.requireFromOutsideHeader(); await proxy.start(home, docs); - homeUrl = serverUrl = await proxy.getServerUrl(); + homeUrl = serverUrl = proxy.serverUrl; hasHomeApi = true; extraHeadersForConfig = { Origin: serverUrl, @@ -281,9 +283,9 @@ describe('DocApi', function () { GRIST_DATA_DIR: dataDir }; home = await TestServer.startServer('home', tmpDir, suitename, additionalEnvConfiguration); - homeUrl = await home.getServerUrl(); + homeUrl = home.serverUrl; docs = await TestServer.startServer('docs', tmpDir, suitename, additionalEnvConfiguration, homeUrl); - serverUrl = await docs.getServerUrl(); + serverUrl = docs.serverUrl; hasHomeApi = false; }); testDocApi({webhooksTestPort}); @@ -3469,7 +3471,7 @@ function testDocApi(settings: { if (docs.proxiedServer) { this.skip(); } - const docWorkerUrl = await docs.getServerUrl(); + const docWorkerUrl = docs.serverUrl; let resp = await axios.get(`${docWorkerUrl}/api/docs/${docIds.Timesheets}/tables/Table1/data`, chimpy); assert.equal(resp.status, 200); assert.containsAllKeys(resp.data, ['A', 'B', 'C']); diff --git a/test/server/lib/ManyFetches.ts b/test/server/lib/ManyFetches.ts index 788f94f1d0..59db25514a 100644 --- a/test/server/lib/ManyFetches.ts +++ b/test/server/lib/ManyFetches.ts @@ -48,8 +48,8 @@ describe('ManyFetches', function() { // Without this limit, there is no pressure on node to garbage-collect, so it may use more // memory than we expect, making the test less reliable. NODE_OPTIONS: '--max-old-space-size=210', - }, await home.getServerUrl()); - userApi = await home.makeUserApi(org, userName); + }, home.serverUrl); + userApi = home.makeUserApi(org, userName); }); afterEach(async function() { @@ -222,13 +222,13 @@ describe('ManyFetches', function() { // function. async function prepareGristWSConnection(docId: string): Promise<() => GristWSConnection> { // Use cookies for access to stay as close as possible to regular operation. - const resp = await fetch(`${await home.getServerUrl()}/test/session`); + const resp = await fetch(`${home.serverUrl}/test/session`); const sid = cookie.parse(resp.headers.get('set-cookie'))[cookieName]; if (!sid) { throw new Error('no session available'); } await home.testingHooks.setLoginSessionProfile(sid, {name: userName, email}, org); // Load the document html. - const pageUrl = `${await home.getServerUrl()}/o/docs/doc/${docId}`; + const pageUrl = `${home.serverUrl}/o/docs/doc/${docId}`; const headers = {Cookie: `${cookieName}=${sid}`}; const doc = await fetch(pageUrl, {headers}); const pageBody = await doc.text(); diff --git a/test/server/lib/UnhandledErrors.ts b/test/server/lib/UnhandledErrors.ts index 84059bc336..a3ef4899a1 100644 --- a/test/server/lib/UnhandledErrors.ts +++ b/test/server/lib/UnhandledErrors.ts @@ -40,7 +40,7 @@ describe('UnhandledErrors', function() { const server = await TestServer.startServer('home', testDir, errType, undefined, undefined, {output}); try { - assert.equal((await fetch(`${await server.getServerUrl()}/status`)).status, 200); + assert.equal((await fetch(`${server.serverUrl}/status`)).status, 200); serverLogLines.length = 0; // Trigger an unhandled error, and check that the server logged it and attempted cleanup. @@ -51,7 +51,7 @@ describe('UnhandledErrors', function() { }, 1000, 100); // We expect the server to be dead now. - await assert.isRejected(fetch(`${await server.getServerUrl()}/status`), /(request.*failed)|(ECONNREFUSED)/); + await assert.isRejected(fetch(`${server.serverUrl}/status`), /(request.*failed)|(ECONNREFUSED)/); } finally { await server.stop(); diff --git a/test/server/lib/Webhooks-Proxy.ts b/test/server/lib/Webhooks-Proxy.ts index 1deb79f1f3..29e78d27d6 100644 --- a/test/server/lib/Webhooks-Proxy.ts +++ b/test/server/lib/Webhooks-Proxy.ts @@ -71,7 +71,7 @@ describe('Webhooks-Proxy', function () { await cb(); // create TestDoc as an empty doc into Private workspace - userApi = api = await home.makeUserApi(ORG_NAME); + userApi = api = home.makeUserApi(ORG_NAME); const wid = await getWorkspaceId(api, 'Private'); docIds.TestDoc = await api.newDoc({name: 'TestDoc'}, wid); }); @@ -125,7 +125,7 @@ describe('Webhooks-Proxy', function () { describe("should work with a merged server", async () => { setupMockServers('merged', tmpDir, async () => { home = docs = await TestServer.startServer('home,docs', tmpDir, suitename, additionaEnvConfiguration); - serverUrl = await home.getServerUrl(); + serverUrl = home.serverUrl; }); subTestCall(); }); @@ -135,7 +135,7 @@ describe('Webhooks-Proxy', function () { describe("should work with a home server and a docworker", async () => { setupMockServers('separated', tmpDir, async () => { home = await TestServer.startServer('home', tmpDir, suitename, additionaEnvConfiguration); - const homeUrl = await home.getServerUrl(); + const homeUrl = home.serverUrl; docs = await TestServer.startServer('docs', tmpDir, suitename, additionaEnvConfiguration, homeUrl); serverUrl = homeUrl; }); @@ -146,9 +146,8 @@ describe('Webhooks-Proxy', function () { describe("should work directly with a docworker", async () => { setupMockServers('docs', tmpDir, async () => { home = await TestServer.startServer('home', tmpDir, suitename, additionaEnvConfiguration); - const homeUrl = await home.getServerUrl(); - docs = await TestServer.startServer('docs', tmpDir, suitename, additionaEnvConfiguration, homeUrl); - serverUrl = await docs.getServerUrl(); + docs = await TestServer.startServer('docs', tmpDir, suitename, additionaEnvConfiguration, home.serverUrl); + serverUrl = docs.serverUrl; }); subTestCall(); }); diff --git a/test/server/lib/helpers/TestServer.ts b/test/server/lib/helpers/TestServer.ts index a462e53fef..0a5c96089e 100644 --- a/test/server/lib/helpers/TestServer.ts +++ b/test/server/lib/helpers/TestServer.ts @@ -12,7 +12,6 @@ import {delay} from "bluebird"; import fetch from "node-fetch"; import {Writable} from "stream"; import express from "express"; -import { AddressInfo } from "net"; import { isAffirmative } from "app/common/gutil"; import httpProxy from 'http-proxy'; @@ -28,8 +27,8 @@ export class TestServer { _homeUrl?: string, options: {output?: Writable} = {}, // Pipe server output to the given stream ): Promise { - - const server = new this(serverTypes, tempDirectory, suitename); + const port = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + const server = new this(serverTypes, port, tempDirectory, suitename); await server.start(_homeUrl, customEnv, options); return server; } @@ -38,15 +37,26 @@ export class TestServer { public testingHooks: TestingHooksClient; public stopped = false; public get proxiedServer() { return this._proxiedServer; } + public get serverUrl() { + if (this._proxiedServer) { + throw new Error('Direct access to this test server is disallowed'); + } + + return `http://localhost:${this.port}`; + } - private _serverUrl: Promise; private _server: ChildProcess; private _exitPromise: Promise; private _proxiedServer: boolean = false; private readonly _defaultEnv; - constructor(private _serverTypes: string, public readonly rootDir: string, private _suiteName: string) { + constructor( + private _serverTypes: string, + public readonly port: number, + public readonly rootDir: string, + private _suiteName: string + ) { this._defaultEnv = { GRIST_INST_DIR: this.rootDir, GRIST_DATA_DIR: path.join(this.rootDir, "data"), @@ -59,18 +69,6 @@ export class TestServer { GRIST_MAX_QUEUE_SIZE: '10', ...process.env }; - this._serverUrl = new Promise((resolve) => { - return getAvailablePort(Number(process.env.GET_AVAILABLE_PORT_START || '8000')).then((port) => { - resolve(`http://localhost:${port}`); - }); - }); - } - - public getServerUrl() { - if (this._proxiedServer) { - throw new Error('Direct access to this test server is disallowed'); - } - return this._serverUrl; } public async start(homeUrl?: string, customEnv?: NodeJS.ProcessEnv, options: {output?: Writable} = {}) { @@ -86,14 +84,11 @@ export class TestServer { throw new Error(`Path of testingSocket too long: ${this.testingSocket.length} (${this.testingSocket})`); } - const serverUrl = await this.getServerUrl(); - const port = new URL(serverUrl).port; - const env: NodeJS.ProcessEnv = { APP_HOME_URL: homeUrl, APP_HOME_INTERNAL_URL: homeUrl, GRIST_TESTING_SOCKET: this.testingSocket, - GRIST_PORT: port, + GRIST_PORT: String(this.port), ...this._defaultEnv, ...customEnv }; @@ -121,7 +116,7 @@ export class TestServer { .catch(() => undefined); await this._waitServerReady(); - log.info(`server ${this._serverTypes} up and listening on ${this._serverUrl}`); + log.info(`server ${this._serverTypes} up and listening on ${this.serverUrl}`); } public async stop() { @@ -150,7 +145,7 @@ export class TestServer { this.testingHooks = await connectTestingHooks(this.testingSocket); // wait for check - return (await fetch(`${this._serverUrl}/status/hooks`, {timeout: 1000})).ok; + return (await fetch(`${this.serverUrl}/status/hooks`, {timeout: 1000})).ok; } catch (err) { log.warn("Failed to initialize server", err); return false; @@ -163,8 +158,8 @@ export class TestServer { // Returns the promise for the ChildProcess's signal or exit code. public getExitPromise(): Promise { return this._exitPromise; } - public async makeUserApi(org: string, user: string = 'chimpy'): Promise { - return new UserAPIImpl(`${await this.getServerUrl()}/o/${org}`, { + public makeUserApi(org: string, user: string = 'chimpy'): UserAPIImpl { + return new UserAPIImpl(`${this.serverUrl}/o/${org}`, { headers: {Authorization: `Bearer api_key_for_${user}`}, fetch: fetch as unknown as typeof globalThis.fetch, newFormData: () => new FormData() as any, @@ -224,21 +219,22 @@ export class TestServerReverseProxy { public static FROM_OUTSIDE_HEADER = {"X-FROM-OUTSIDE": true}; + public static async build() { + const port = await getAvailablePort(parseInt(process.env.GET_AVAILABLE_PORT_START || '8080', 10)); + return new this(port); + } + + public get serverUrl() { return `http://${TestServerReverseProxy.HOSTNAME}:${this.port}`; } + private _app = express(); private _proxyServer: http.Server; private _proxy: httpProxy = httpProxy.createProxy(); - private _address: Promise; private _requireFromOutsideHeader = false; public get stopped() { return !this._proxyServer.listening; } - public constructor() { - this._proxyServer = this._app.listen(0); - this._address = new Promise((resolve) => { - this._proxyServer.once('listening', () => { - resolve(this._proxyServer.address() as AddressInfo); - }); - }); + public constructor(public readonly port: number) { + this._proxyServer = this._app.listen(port); } /** @@ -262,16 +258,7 @@ export class TestServerReverseProxy { homeServer.disallowDirectAccess(); docServer.disallowDirectAccess(); - log.info('proxy server running on ', await this.getServerUrl()); - } - - public async getAddress() { - return this._address; - } - - public async getServerUrl() { - const address = await this.getAddress(); - return `http://${TestServerReverseProxy.HOSTNAME}:${address.port}`; + log.info('proxy server running on ', this.serverUrl); } public stop() { @@ -284,7 +271,7 @@ export class TestServerReverseProxy { } private async _getRequestHandlerFor(server: TestServer) { - const serverUrl = new URL(await server.getServerUrl()); + const serverUrl = new URL(server.serverUrl); return (oreq: express.Request, ores: express.Response) => { log.debug(`[proxy] Requesting (method=${oreq.method}): ${new URL(oreq.url, serverUrl).href}`);