-
Notifications
You must be signed in to change notification settings - Fork 59
Add proxy integration tests and missing UTS test coverage #2216
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: uts-integration
Are you sure you want to change the base?
Changes from 13 commits
6cebb50
6730b98
2953ebe
89ad6a2
e0c1345
774ed58
f9f6166
b61e070
0302d6b
743e8a3
910c107
7444c39
0e3b974
3f87a4b
09a3ac9
73f6568
ac8a387
184c10f
71a0297
3eaa563
b94191d
eacbc38
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,294 @@ | ||
| /** | ||
| * TypeScript helper for the Go test proxy. | ||
| * | ||
| * Wraps the proxy's REST control API to create sessions, add rules, | ||
| * trigger imperative actions, retrieve event logs, and clean up. | ||
| * | ||
| * The proxy binary is downloaded from GitHub releases on first use | ||
| * via ensureProxy(). It is killed when the Node.js process exits. | ||
| */ | ||
|
|
||
| import { execSync, spawn, ChildProcess } from 'child_process'; | ||
| import * as crypto from 'crypto'; | ||
| import * as path from 'path'; | ||
| import * as fs from 'fs'; | ||
| import { pipeline } from 'stream/promises'; | ||
|
|
||
| const PROXY_VERSION = 'v0.1.0'; | ||
| const PROXY_REPO = 'ably/uts-proxy'; | ||
|
|
||
| const CONTROL_PORT = process.env.PROXY_CONTROL_PORT || '9100'; | ||
| const PROXY_CONTROL_HOST = process.env.PROXY_CONTROL_HOST || `http://localhost:${CONTROL_PORT}`; | ||
| const CACHE_DIR = path.resolve(__dirname, '../../../../../node_modules/.cache/uts-proxy', PROXY_VERSION); | ||
| const PROXY_BIN = path.join(CACHE_DIR, 'uts-proxy'); | ||
|
|
||
| let _proxyProcess: ChildProcess | null = null; | ||
| let _proxyEnsured = false; | ||
|
|
||
| const SANDBOX_REALTIME_HOST = 'sandbox-realtime.ably.io'; | ||
| const SANDBOX_REST_HOST = 'sandbox-rest.ably.io'; | ||
|
|
||
| let nextPort = 19000 + Math.floor(Math.random() * 1000); | ||
|
|
||
| function allocatePort(): number { | ||
| return nextPort++; | ||
| } | ||
|
|
||
| interface ProxyRule { | ||
| match: { | ||
| type: string; | ||
| count?: number; | ||
| action?: string; | ||
| channel?: string; | ||
| method?: string; | ||
| pathContains?: string; | ||
| queryContains?: Record<string, string>; | ||
| delayMs?: number; | ||
| }; | ||
| action: { | ||
| type: string; | ||
| closeCode?: number; | ||
| delayMs?: number; | ||
| message?: Record<string, any>; | ||
| status?: number; | ||
| body?: Record<string, any>; | ||
| headers?: Record<string, string>; | ||
| }; | ||
| times?: number; | ||
| comment?: string; | ||
| } | ||
|
|
||
| interface ProxyEvent { | ||
| timestamp: string; | ||
| type: string; | ||
| direction?: string; | ||
| url?: string; | ||
| queryParams?: Record<string, string>; | ||
| message?: any; | ||
| method?: string; | ||
| path?: string; | ||
| status?: number; | ||
| initiator?: string; | ||
| closeCode?: number; | ||
| ruleMatched?: string | null; | ||
| headers?: Record<string, string>; | ||
| } | ||
|
|
||
| interface ImperativeAction { | ||
| type: string; | ||
| message?: Record<string, any>; | ||
| closeCode?: number; | ||
| } | ||
|
|
||
| class ProxySession { | ||
| readonly sessionId: string; | ||
| readonly proxyHost: string; | ||
| readonly proxyPort: number; | ||
| private controlUrl: string; | ||
|
|
||
| constructor(sessionId: string, proxyHost: string, proxyPort: number, controlUrl: string) { | ||
| this.sessionId = sessionId; | ||
| this.proxyHost = proxyHost; | ||
| this.proxyPort = proxyPort; | ||
| this.controlUrl = controlUrl; | ||
| } | ||
|
|
||
| async addRules(rules: ProxyRule[], position: 'append' | 'prepend' = 'append'): Promise<void> { | ||
| const resp = await fetch(`${this.controlUrl}/sessions/${this.sessionId}/rules`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ rules, position }), | ||
| }); | ||
| if (!resp.ok) { | ||
| const body = await resp.text(); | ||
| throw new Error(`addRules failed (${resp.status}): ${body}`); | ||
| } | ||
| } | ||
|
|
||
| async triggerAction(action: ImperativeAction): Promise<void> { | ||
| const resp = await fetch(`${this.controlUrl}/sessions/${this.sessionId}/actions`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(action), | ||
| }); | ||
| if (!resp.ok) { | ||
| const body = await resp.text(); | ||
| throw new Error(`triggerAction failed (${resp.status}): ${body}`); | ||
| } | ||
| } | ||
|
|
||
| async getLog(): Promise<ProxyEvent[]> { | ||
| const resp = await fetch(`${this.controlUrl}/sessions/${this.sessionId}/log`); | ||
| if (!resp.ok) { | ||
| const body = await resp.text(); | ||
| throw new Error(`getLog failed (${resp.status}): ${body}`); | ||
| } | ||
| const data = await resp.json(); | ||
| return data.events || []; | ||
|
Comment on lines
+97
to
+127
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: No, the Node.js global fetch does not have a single default request timeout parameter in its API. Instead, the underlying Undici implementation enforces specific timeouts: headersTimeout (300 seconds / 5 minutes, time to receive headers), bodyTimeout (300 seconds, time between body chunks), and connectTimeout (around 10 seconds in some cases). These can cause requests to fail with errors like HeadersTimeoutError or ConnectTimeoutError if exceeded, even without an explicit user-set timeout. AbortSignal.timeout(ms) creates an AbortSignal that automatically aborts after the specified milliseconds with a TimeoutError. Pass it via the signal option to cap the entire request duration, overriding or complementing Undici's internal timeouts: try { const response = await fetch('https://example.com', { signal: AbortSignal.timeout(5000) // 5 second cap }); // handle response } catch (err) { if (err.name === 'TimeoutError') { console.log('Request timed out after 5s'); } } This is supported in Node.js since v17.3.0 (AbortSignal.timeout added v17.3.0/v16.14.0; stable fetch since v18). For finer control over Undici timeouts (e.g., increasing headersTimeout), use a custom Agent dispatcher: import { Agent } from 'undici'; await fetch(url, { dispatcher: new Agent({ headersTimeout: 600e3 }) }); Citations:
🏁 Script executed: # Find the proxy.ts file and examine the ensureProxy method around line 263
find . -type f -name "proxy.ts" | head -5Repository: ably/ably-js Length of output: 105 🏁 Script executed: # Check the file to understand the ensureProxy method and timeoutMs usage
cat -n test/uts/realtime/integration/helpers/proxy.ts | sed -n '250,270p'Repository: ably/ably-js Length of output: 671 🏁 Script executed: # Also check the full context of ensureProxy method
cat -n test/uts/realtime/integration/helpers/proxy.ts | sed -n '250,300p'Repository: ably/ably-js Length of output: 1462 🏁 Script executed: # Check other fetch calls mentioned (lines 132, 164-168, 206-208, 251-267)
cat -n test/uts/realtime/integration/helpers/proxy.ts | sed -n '1,50p'Repository: ably/ably-js Length of output: 1910 🏁 Script executed: # Check the other fetch calls mentioned in the review
cat -n test/uts/realtime/integration/helpers/proxy.ts | sed -n '95,135p'Repository: ably/ably-js Length of output: 1731 🏁 Script executed: # Check lines around 164-168 and 206-208
cat -n test/uts/realtime/integration/helpers/proxy.ts | sed -n '160,210p'Repository: ably/ably-js Length of output: 2223 Add per-request timeouts to all control-plane fetch calls. These Use Affects: 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| async close(): Promise<void> { | ||
| try { | ||
| await fetch(`${this.controlUrl}/sessions/${this.sessionId}`, { method: 'DELETE' }); | ||
| } catch { | ||
| // Ignore errors during cleanup | ||
| } | ||
| } | ||
| } | ||
|
|
||
| interface CreateProxySessionOpts { | ||
| endpoint?: 'sandbox'; | ||
| port?: number; | ||
| rules?: ProxyRule[]; | ||
| timeoutMs?: number; | ||
| } | ||
|
|
||
| async function createProxySession(opts: CreateProxySessionOpts = {}): Promise<ProxySession> { | ||
| const port = opts.port || allocatePort(); | ||
| const controlUrl = PROXY_CONTROL_HOST; | ||
|
|
||
| const target = { | ||
| realtimeHost: SANDBOX_REALTIME_HOST, | ||
| restHost: SANDBOX_REST_HOST, | ||
| }; | ||
|
|
||
| const body: Record<string, any> = { | ||
| target, | ||
| port, | ||
| rules: opts.rules || [], | ||
| }; | ||
| if (opts.timeoutMs) { | ||
| body.timeoutMs = opts.timeoutMs; | ||
| } | ||
|
|
||
| const resp = await fetch(`${controlUrl}/sessions`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify(body), | ||
| }); | ||
|
|
||
| if (!resp.ok) { | ||
| const text = await resp.text(); | ||
| throw new Error(`createProxySession failed (${resp.status}): ${text}`); | ||
| } | ||
|
|
||
| const data = await resp.json(); | ||
| return new ProxySession(data.sessionId, 'localhost', port, controlUrl); | ||
| } | ||
|
|
||
| const CHECKSUMS: Record<string, string> = { | ||
| 'uts-proxy_darwin_amd64.tar.gz': 'eb8abf5eec7f7137cf9e7cb6ab6f45fd162303c242b4567ab9e354c4b9a4a4ff', | ||
| 'uts-proxy_darwin_arm64.tar.gz': '845da80af7d5b1daacbdf30b34aff6ca1b2bb88c708065bdc5d9a636baf32a1f', | ||
| 'uts-proxy_linux_amd64.tar.gz': '79f444c23362cc277d163deb243dc16063c74665ff63b8bd3e56789b9d9610c7', | ||
| 'uts-proxy_linux_arm64.tar.gz': '7357e4605f19451d83bb419ee959537d6e95ca74b766721eae006d4171371030', | ||
| }; | ||
|
|
||
| function assetName(): string { | ||
| const platform = process.platform === 'darwin' ? 'darwin' : 'linux'; | ||
| const arch = process.arch === 'arm64' ? 'arm64' : 'amd64'; | ||
| return `uts-proxy_${platform}_${arch}.tar.gz`; | ||
| } | ||
|
Comment on lines
+179
to
+190
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Node.js process.platform returns one of the following string values: 'aix', 'darwin', 'freebsd', 'linux', 'openbsd', 'sunos', or 'win32'. The value 'android' may also be returned if Node.js is built on Android (experimental support). Windows is identified by the value 'win32' (regardless of 32-bit or 64-bit architecture). Citations:
Handle unsupported platforms explicitly. The 🤖 Prompt for AI Agents |
||
|
|
||
| async function downloadProxy(): Promise<void> { | ||
| if (fs.existsSync(PROXY_BIN)) return; | ||
|
|
||
| const asset = assetName(); | ||
| const expectedHash = CHECKSUMS[asset]; | ||
| if (!expectedHash) { | ||
| throw new Error(`No checksum for ${asset} — unsupported platform/arch`); | ||
| } | ||
|
|
||
| fs.mkdirSync(CACHE_DIR, { recursive: true }); | ||
|
|
||
| const url = `https://github.com/${PROXY_REPO}/releases/download/${PROXY_VERSION}/${asset}`; | ||
| console.log(`Downloading uts-proxy ${PROXY_VERSION} (${asset})...`); | ||
|
|
||
| const resp = await fetch(url, { redirect: 'follow' }); | ||
| if (!resp.ok || !resp.body) { | ||
| throw new Error(`Failed to download ${url}: ${resp.status} ${resp.statusText}`); | ||
| } | ||
|
|
||
| const tarball = path.join(CACHE_DIR, asset); | ||
| const fileStream = fs.createWriteStream(tarball); | ||
| // @ts-ignore — Node fetch body is a web ReadableStream; pipeline handles it in Node 18+ | ||
| await pipeline(resp.body, fileStream); | ||
|
|
||
| const hash = crypto.createHash('sha256').update(fs.readFileSync(tarball)).digest('hex'); | ||
| if (hash !== expectedHash) { | ||
| fs.unlinkSync(tarball); | ||
| throw new Error(`Checksum mismatch for ${asset}: expected ${expectedHash}, got ${hash}`); | ||
| } | ||
|
|
||
| execSync(`tar xzf ${JSON.stringify(asset)}`, { cwd: CACHE_DIR }); | ||
| fs.chmodSync(PROXY_BIN, 0o755); | ||
| fs.unlinkSync(tarball); | ||
| } | ||
|
|
||
| function spawnProxy(): ChildProcess { | ||
| const child = spawn(PROXY_BIN, ['--port', CONTROL_PORT], { | ||
| stdio: ['ignore', 'inherit', 'inherit'], | ||
| detached: false, | ||
| }); | ||
|
|
||
| child.on('error', (err) => { | ||
| console.error(`Proxy process error: ${err.message}`); | ||
| }); | ||
|
|
||
| process.on('exit', () => { | ||
| if (child.exitCode === null) { | ||
| child.kill(); | ||
| } | ||
| }); | ||
|
|
||
| return child; | ||
| } | ||
|
|
||
| async function ensureProxy(timeoutMs = 15000): Promise<void> { | ||
| if (_proxyEnsured) return; | ||
|
|
||
| // Check if proxy is already running (e.g. started externally) | ||
| try { | ||
| const resp = await fetch(`${PROXY_CONTROL_HOST}/health`); | ||
| if (resp.ok) { | ||
| _proxyEnsured = true; | ||
| return; | ||
| } | ||
| } catch { | ||
| // Not running — we'll start it | ||
| } | ||
|
|
||
| await downloadProxy(); | ||
| _proxyProcess = spawnProxy(); | ||
|
|
||
| const start = Date.now(); | ||
| while (Date.now() - start < timeoutMs) { | ||
| try { | ||
| const resp = await fetch(`${PROXY_CONTROL_HOST}/health`); | ||
| if (resp.ok) { | ||
| _proxyEnsured = true; | ||
| return; | ||
| } | ||
| } catch { | ||
| // Not ready yet | ||
| } | ||
| await new Promise((r) => setTimeout(r, 200)); | ||
| } | ||
|
|
||
| _proxyProcess.kill(); | ||
| _proxyProcess = null; | ||
| throw new Error(`Proxy failed to start within ${timeoutMs}ms`); | ||
|
Comment on lines
+246
to
+279
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Memoize proxy startup instead of racing it. Two callers can pass Line 247 before Possible shape of the fix let _proxyProcess: ChildProcess | null = null;
let _proxyEnsured = false;
+let _proxyEnsurePromise: Promise<void> | null = null;
async function ensureProxy(timeoutMs = 15000): Promise<void> {
if (_proxyEnsured) return;
+ if (_proxyEnsurePromise) return _proxyEnsurePromise;
- // existing startup logic
+ _proxyEnsurePromise = (async () => {
+ // existing startup logic
+ })();
+
+ try {
+ await _proxyEnsurePromise;
+ } finally {
+ _proxyEnsurePromise = null;
+ }
}🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| async function waitForProxy(timeoutMs = 15000): Promise<void> { | ||
| await ensureProxy(timeoutMs); | ||
| } | ||
|
|
||
| function stopProxy(): void { | ||
| if (_proxyProcess && _proxyProcess.exitCode === null) { | ||
| _proxyProcess.kill(); | ||
| _proxyProcess = null; | ||
| } | ||
| _proxyEnsured = false; | ||
| } | ||
|
|
||
| export { ProxySession, ProxyRule, ProxyEvent, ImperativeAction, createProxySession, waitForProxy, ensureProxy, stopProxy, allocatePort }; | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Stop hand-allocating session ports.
nextPort++never checks or reserves the socket, so parallel workers or anything already bound in the 19000-19999 range can makecreateProxySession()fail nondeterministically. This needs OS-backed allocation, or proxy-side ephemeral port assignment, instead of a process-local counter.Also applies to: 146-176
🤖 Prompt for AI Agents