From 7fe02a4bddf95303dcb2a9289bca5dbf645dfe32 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 11 Jan 2024 20:27:51 +0100 Subject: [PATCH 1/5] test(node): Test proxy server --- .../node-integration-tests/package.json | 1 + .../suites/proxy/basic.js | 18 +++ .../suites/proxy/test.ts | 12 ++ .../node-integration-tests/utils/runner.ts | 149 +++++++++++------- .../node-integration-tests/utils/server.ts | 34 ++++ yarn.lock | 39 +++++ 6 files changed, 193 insertions(+), 60 deletions(-) create mode 100644 dev-packages/node-integration-tests/suites/proxy/basic.js create mode 100644 dev-packages/node-integration-tests/suites/proxy/test.ts create mode 100644 dev-packages/node-integration-tests/utils/server.ts diff --git a/dev-packages/node-integration-tests/package.json b/dev-packages/node-integration-tests/package.json index d9f292a9be4f..450707a465d7 100644 --- a/dev-packages/node-integration-tests/package.json +++ b/dev-packages/node-integration-tests/package.json @@ -44,6 +44,7 @@ "mysql": "^2.18.1", "nock": "^13.1.0", "pg": "^8.7.3", + "proxy": "^2.1.1", "yargs": "^16.2.0" }, "config": { diff --git a/dev-packages/node-integration-tests/suites/proxy/basic.js b/dev-packages/node-integration-tests/suites/proxy/basic.js new file mode 100644 index 000000000000..37c568ee613d --- /dev/null +++ b/dev-packages/node-integration-tests/suites/proxy/basic.js @@ -0,0 +1,18 @@ +const http = require('http'); +const Sentry = require('@sentry/node'); +const { createProxy } = require('proxy'); + +const proxy = createProxy(http.createServer()); +proxy.listen(0, () => { + const proxyPort = proxy.address().port; + + Sentry.init({ + dsn: process.env.SENTRY_DSN, + debug: true, + transportOptions: { + proxy: `http://localhost:${proxyPort}`, + }, + }); + + Sentry.captureMessage('Hello, via proxy!'); +}); diff --git a/dev-packages/node-integration-tests/suites/proxy/test.ts b/dev-packages/node-integration-tests/suites/proxy/test.ts new file mode 100644 index 000000000000..82e4c1145ce4 --- /dev/null +++ b/dev-packages/node-integration-tests/suites/proxy/test.ts @@ -0,0 +1,12 @@ +import { createRunner } from '../../utils/runner'; + +test('proxies sentry requests', done => { + createRunner(__dirname, 'basic.js') + .withMockSentryServer() + .expect({ + event: { + message: 'Hello, via proxy!', + }, + }) + .start(done); +}, 10000); diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index 2b89c9deb46f..a857327e79af 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -2,6 +2,7 @@ import { spawn } from 'child_process'; import { join } from 'path'; import type { Envelope, EnvelopeItemType, Event, SerializedSession } from '@sentry/types'; import axios from 'axios'; +import { createBasicSentryServer } from './server'; export function assertSentryEvent(actual: Event, expected: Event): void { expect(actual).toMatchObject({ @@ -28,6 +29,18 @@ export function assertSentryTransaction(actual: Event, expected: Partial) }); } +/** Promise only resolves when fn returns true */ +async function waitFor(fn: () => boolean, timeout = 10_000): Promise { + let remaining = timeout; + while (fn() === false) { + await new Promise(resolve => setTimeout(resolve, 100)); + remaining -= 100; + if (remaining < 0) { + throw new Error('Timed out waiting for server port'); + } + } +} + type Expected = | { event: Partial | ((event: Event) => void); @@ -39,7 +52,7 @@ type Expected = session: Partial | ((event: SerializedSession) => void); }; -/** */ +/** Creates a test runner */ // eslint-disable-next-line @typescript-eslint/explicit-function-return-type export function createRunner(...paths: string[]) { const testPath = join(...paths); @@ -47,7 +60,7 @@ export function createRunner(...paths: string[]) { const expectedEnvelopes: Expected[] = []; const flags: string[] = []; const ignored: EnvelopeItemType[] = []; - let hasExited = false; + let withSentryServer = false; if (testPath.endsWith('.ts')) { flags.push('-r', 'ts-node/register'); @@ -62,69 +75,32 @@ export function createRunner(...paths: string[]) { flags.push(...args); return this; }, + withMockSentryServer: function () { + withSentryServer = true; + return this; + }, ignore: function (...types: EnvelopeItemType[]) { ignored.push(...types); return this; }, start: function (done?: (e?: unknown) => void) { const expectedEnvelopeCount = expectedEnvelopes.length; - let envelopeCount = 0; - let serverPort: number | undefined; - - const child = spawn('node', [...flags, testPath]); - child.on('close', () => { - hasExited = true; - }); - - // Pass error to done to end the test quickly - child.on('error', e => { - done?.(e); - }); - - async function waitForServerPort(timeout = 10_000): Promise { - let remaining = timeout; - while (serverPort === undefined) { - await new Promise(resolve => setTimeout(resolve, 100)); - remaining -= 100; - if (remaining < 0) { - throw new Error('Timed out waiting for server port'); - } - } - } + let envelopeCount = 0; + let scenarioServerPort: number | undefined; + let hasExited = false; + let child: ReturnType | undefined; /** Called after each expect callback to check if we're complete */ function expectCallbackCalled(): void { envelopeCount++; if (envelopeCount === expectedEnvelopeCount) { - child.kill(); + child?.kill(); done?.(); } } - function tryParseLine(line: string): void { - // Lines can have leading '[something] [{' which we need to remove - const cleanedLine = line.replace(/^.*?] \[{"/, '[{"'); - - // See if we have a port message - if (cleanedLine.startsWith('{"port":')) { - const { port } = JSON.parse(cleanedLine) as { port: number }; - serverPort = port; - return; - } - - // Skip any lines that don't start with envelope JSON - if (!cleanedLine.startsWith('[{')) { - return; - } - - let envelope: Envelope | undefined; - try { - envelope = JSON.parse(cleanedLine) as Envelope; - } catch (_) { - return; - } - + function newEnvelope(envelope: Envelope): void { for (const item of envelope[1]) { const envelopeItemType = item[0].type; @@ -184,17 +160,70 @@ export function createRunner(...paths: string[]) { } } - let buffer = Buffer.alloc(0); - child.stdout.on('data', (data: Buffer) => { - // This is horribly memory inefficient but it's only for tests - buffer = Buffer.concat([buffer, data]); + const serverStartup: Promise = withSentryServer + ? createBasicSentryServer(newEnvelope) + : Promise.resolve(undefined); + + // eslint-disable-next-line @typescript-eslint/no-floating-promises + serverStartup.then(mockServerPort => { + const env = mockServerPort + ? { ...process.env, SENTRY_DSN: `http://public@localhost:${mockServerPort}/1337` } + : process.env; + + // eslint-disable-next-line no-console + if (process.env.DEBUG) console.log('starting scenario', testPath, flags, env.SENTRY_DSN); + + child = spawn('node', [...flags, testPath], { env }); + + child.on('close', () => { + hasExited = true; + }); + + // Pass error to done to end the test quickly + child.on('error', e => { + // eslint-disable-next-line no-console + if (process.env.DEBUG) console.log('scenario error', e); + done?.(e); + }); + + function tryParseEnvelopeFromStdoutLine(line: string): void { + // Lines can have leading '[something] [{' which we need to remove + const cleanedLine = line.replace(/^.*?] \[{"/, '[{"'); + + // See if we have a port message + if (cleanedLine.startsWith('{"port":')) { + const { port } = JSON.parse(cleanedLine) as { port: number }; + scenarioServerPort = port; + return; + } + + // Skip any lines that don't start with envelope JSON + if (!cleanedLine.startsWith('[{')) { + return; + } - let splitIndex = -1; - while ((splitIndex = buffer.indexOf(0xa)) >= 0) { - const line = buffer.subarray(0, splitIndex).toString(); - buffer = Buffer.from(buffer.subarray(splitIndex + 1)); - tryParseLine(line); + try { + const envelope = JSON.parse(cleanedLine) as Envelope; + newEnvelope(envelope); + } catch (_) { + // + } } + + let buffer = Buffer.alloc(0); + child.stdout.on('data', (data: Buffer) => { + // This is horribly memory inefficient but it's only for tests + buffer = Buffer.concat([buffer, data]); + + let splitIndex = -1; + while ((splitIndex = buffer.indexOf(0xa)) >= 0) { + const line = buffer.subarray(0, splitIndex).toString(); + buffer = Buffer.from(buffer.subarray(splitIndex + 1)); + // eslint-disable-next-line no-console + if (process.env.DEBUG) console.log('line', line); + tryParseEnvelopeFromStdoutLine(line); + } + }); }); return { @@ -207,9 +236,9 @@ export function createRunner(...paths: string[]) { headers: Record = {}, ): Promise { try { - await waitForServerPort(); + await waitFor(() => scenarioServerPort !== undefined); - const url = `http://localhost:${serverPort}${path}`; + const url = `http://localhost:${scenarioServerPort}${path}`; if (method === 'get') { return (await axios.get(url, { headers })).data; } else { diff --git a/dev-packages/node-integration-tests/utils/server.ts b/dev-packages/node-integration-tests/utils/server.ts new file mode 100644 index 000000000000..933d07d8741f --- /dev/null +++ b/dev-packages/node-integration-tests/utils/server.ts @@ -0,0 +1,34 @@ +import type { AddressInfo } from 'net'; +import { TextDecoder, TextEncoder } from 'util'; +import type { Envelope } from '@sentry/types'; +import { parseEnvelope } from '@sentry/utils'; +import express from 'express'; + +/** + * Creates a basic Sentry server that accepts POST to the envelope endpoint + * + * This does no checks on the envelope, it just calls the callback if it managed to parse an envelope from the raw POST + * body data. + */ +export function createBasicSentryServer(onEnvelope: (env: Envelope) => void): Promise { + const app = express(); + app.use(express.raw({ type: () => true, inflate: true, limit: '100mb' })); + app.post('/api/:id/envelope/', (req, res) => { + try { + const env = parseEnvelope(req.body as Buffer, new TextEncoder(), new TextDecoder()); + onEnvelope(env); + } catch (e) { + // eslint-disable-next-line no-console + console.error(e); + } + + res.status(200).send(); + }); + + return new Promise(resolve => { + const server = app.listen(0, () => { + const address = server.address() as AddressInfo; + resolve(address.port); + }); + }); +} diff --git a/yarn.lock b/yarn.lock index 3f7c2a8141c7..db9bf4e6004a 100644 --- a/yarn.lock +++ b/yarn.lock @@ -8051,6 +8051,16 @@ argparse@^2.0.1: resolved "https://registry.yarnpkg.com/argparse/-/argparse-2.0.1.tgz#246f50f3ca78a3240f6c997e8a9bd1eac49e4b38" integrity sha512-8+9WqebbFzpX9OR+Wa6O29asIogeRMzcGtAINdpMHHyAg10f05aSFVBbcEqGf/PXw1EjAZ+q2/bEBg3DvurK3Q== +args@^5.0.3: + version "5.0.3" + resolved "https://registry.yarnpkg.com/args/-/args-5.0.3.tgz#943256db85021a85684be2f0882f25d796278702" + integrity sha512-h6k/zfFgusnv3i5TU08KQkVKuCPBtL/PWQbWkHUxvJrZ2nAyeaUupneemcrgn1xmqxPQsPIzwkUhOpoqPDRZuA== + dependencies: + camelcase "5.0.0" + chalk "2.4.2" + leven "2.1.0" + mri "1.1.4" + argv@0.0.2: version "0.0.2" resolved "https://registry.yarnpkg.com/argv/-/argv-0.0.2.tgz#ecbd16f8949b157183711b1bda334f37840185ab" @@ -9428,6 +9438,11 @@ base@^0.11.1: mixin-deep "^1.2.0" pascalcase "^0.1.1" +basic-auth-parser@0.0.2-1: + version "0.0.2-1" + resolved "https://registry.yarnpkg.com/basic-auth-parser/-/basic-auth-parser-0.0.2-1.tgz#f1ea575979b27af6a411921d6ff8793d9117347f" + integrity sha512-GFj8iVxo9onSU6BnnQvVwqvxh60UcSHJEDnIk3z4B6iOjsKSmqe+ibW0Rsz7YO7IE1HG3D3tqCNIidP46SZVdQ== + basic-auth@~2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/basic-auth/-/basic-auth-2.0.1.tgz#b998279bf47ce38344b4f3cf916d4679bbf51e3a" @@ -10703,6 +10718,11 @@ camelcase-keys@^6.2.2: map-obj "^4.0.0" quick-lru "^4.0.1" +camelcase@5.0.0: + version "5.0.0" + resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-5.0.0.tgz#03295527d58bd3cd4aa75363f35b2e8d97be2f42" + integrity sha512-faqwZqnWxbxn+F1d399ygeamQNy3lPp/H9H6rNrqYh4FSVCtcY+3cub1MxA8o9mDd55mM8Aghuu/kuyYA6VTsA== + camelcase@5.3.1, camelcase@^5.0.0, camelcase@^5.3.1: version "5.3.1" resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-5.3.1.tgz#e3c9b31569e106811df242f715725a1f4c494320" @@ -20024,6 +20044,11 @@ less@^4.1.0: needle "^3.1.0" source-map "~0.6.0" +leven@2.1.0: + version "2.1.0" + resolved "https://registry.yarnpkg.com/leven/-/leven-2.1.0.tgz#c2e7a9f772094dee9d34202ae8acce4687875580" + integrity sha512-nvVPLpIHUxCUoRLrFqTgSxXJ614d8AgQoWl7zPe/2VadE8+1dpU3LBhowRuBAcuwruWtOdD8oYC9jDNJjXDPyA== + leven@^3.1.0: version "3.1.0" resolved "https://registry.yarnpkg.com/leven/-/leven-3.1.0.tgz#77891de834064cccba82ae7842bb6b14a13ed7f2" @@ -22141,6 +22166,11 @@ move-concurrently@^1.0.1: rimraf "^2.5.4" run-queue "^1.0.3" +mri@1.1.4: + version "1.1.4" + resolved "https://registry.yarnpkg.com/mri/-/mri-1.1.4.tgz#7cb1dd1b9b40905f1fac053abe25b6720f44744a" + integrity sha512-6y7IjGPm8AzlvoUrwAaw1tLnUBudaS3752vcd8JtrpGGQn+rXIe63LFVHm/YMwtqAuh+LJPCFdlLYPWM1nYn6w== + mri@^1.1.0: version "1.2.0" resolved "https://registry.yarnpkg.com/mri/-/mri-1.2.0.tgz#6721480fec2a11a4889861115a48b6cbe7cc8f0b" @@ -25893,6 +25923,15 @@ proxy-from-env@^1.1.0: resolved "https://registry.yarnpkg.com/proxy-from-env/-/proxy-from-env-1.1.0.tgz#e102f16ca355424865755d2c9e8ea4f24d58c3e2" integrity sha512-D+zkORCbA9f1tdWRK0RaCR3GPv50cMxcrz4X8k5LTSUD1Dkw47mKJEZQNunItRTkWwgtaUSo1RVFRIG9ZXiFYg== +proxy@^2.1.1: + version "2.1.1" + resolved "https://registry.yarnpkg.com/proxy/-/proxy-2.1.1.tgz#45f9b307508ffcae12bdc71678d44a4ab79cbf8b" + integrity sha512-nLgd7zdUAOpB3ZO/xCkU8gy74UER7P0aihU8DkUsDS5ZoFwVCX7u8dy+cv5tVK8UaB/yminU1GiLWE26TKPYpg== + dependencies: + args "^5.0.3" + basic-auth-parser "0.0.2-1" + debug "^4.3.4" + prr@~1.0.1: version "1.0.1" resolved "https://registry.yarnpkg.com/prr/-/prr-1.0.1.tgz#d3fc114ba06995a45ec6893f484ceb1d78f5f476" From 4afeef759aadbf05f66c0e3f36e4edf7102f0b89 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 11 Jan 2024 20:54:01 +0100 Subject: [PATCH 2/5] lint --- MIGRATION.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MIGRATION.md b/MIGRATION.md index d3f2047deb27..fe0336d2d1f6 100644 --- a/MIGRATION.md +++ b/MIGRATION.md @@ -66,7 +66,9 @@ If you are using the `Hub` right now, see the following table on how to migrate ## Deprecate `client.setupIntegrations()` -Instead, use the new `client.init()` method. You should probably not use this directly and instead use `Sentry.init()`, which calls this under the hood. But if you have a special use case that requires that, you can call `client.init()` instead now. +Instead, use the new `client.init()` method. You should probably not use this directly and instead use `Sentry.init()`, +which calls this under the hood. But if you have a special use case that requires that, you can call `client.init()` +instead now. ## Deprecate `scope.getSpan()` and `scope.setSpan()` From ba729293c22618f2488744bf960720f82a7c5181 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 11 Jan 2024 21:02:29 +0100 Subject: [PATCH 3/5] extend timeout --- dev-packages/node-integration-tests/suites/proxy/test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/node-integration-tests/suites/proxy/test.ts b/dev-packages/node-integration-tests/suites/proxy/test.ts index 82e4c1145ce4..be104a4d0871 100644 --- a/dev-packages/node-integration-tests/suites/proxy/test.ts +++ b/dev-packages/node-integration-tests/suites/proxy/test.ts @@ -9,4 +9,4 @@ test('proxies sentry requests', done => { }, }) .start(done); -}, 10000); +}); From 6d1a44741d1562caf3803f60b91ed82001f9f8e9 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Thu, 11 Jan 2024 21:17:34 +0100 Subject: [PATCH 4/5] ignore sessions --- dev-packages/node-integration-tests/suites/proxy/test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/dev-packages/node-integration-tests/suites/proxy/test.ts b/dev-packages/node-integration-tests/suites/proxy/test.ts index be104a4d0871..5e4619d3948d 100644 --- a/dev-packages/node-integration-tests/suites/proxy/test.ts +++ b/dev-packages/node-integration-tests/suites/proxy/test.ts @@ -3,6 +3,7 @@ import { createRunner } from '../../utils/runner'; test('proxies sentry requests', done => { createRunner(__dirname, 'basic.js') .withMockSentryServer() + .ignore('session') .expect({ event: { message: 'Hello, via proxy!', From 74b347c683cc39703c42fa16bf13a3bca10fa32a Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Sat, 13 Jan 2024 06:46:46 +0100 Subject: [PATCH 5/5] complete --- .../node-integration-tests/utils/runner.ts | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/dev-packages/node-integration-tests/utils/runner.ts b/dev-packages/node-integration-tests/utils/runner.ts index a857327e79af..de3b0e962975 100644 --- a/dev-packages/node-integration-tests/utils/runner.ts +++ b/dev-packages/node-integration-tests/utils/runner.ts @@ -91,12 +91,16 @@ export function createRunner(...paths: string[]) { let hasExited = false; let child: ReturnType | undefined; + function complete(error?: Error): void { + child?.kill(); + done?.(error); + } + /** Called after each expect callback to check if we're complete */ function expectCallbackCalled(): void { envelopeCount++; if (envelopeCount === expectedEnvelopeCount) { - child?.kill(); - done?.(); + complete(); } } @@ -155,7 +159,7 @@ export function createRunner(...paths: string[]) { expectCallbackCalled(); } } catch (e) { - done?.(e); + complete(e as Error); } } } @@ -183,7 +187,7 @@ export function createRunner(...paths: string[]) { child.on('error', e => { // eslint-disable-next-line no-console if (process.env.DEBUG) console.log('scenario error', e); - done?.(e); + complete(e); }); function tryParseEnvelopeFromStdoutLine(line: string): void { @@ -237,17 +241,17 @@ export function createRunner(...paths: string[]) { ): Promise { try { await waitFor(() => scenarioServerPort !== undefined); - - const url = `http://localhost:${scenarioServerPort}${path}`; - if (method === 'get') { - return (await axios.get(url, { headers })).data; - } else { - return (await axios.post(url, { headers })).data; - } } catch (e) { - done?.(e); + complete(e as Error); return undefined; } + + const url = `http://localhost:${scenarioServerPort}${path}`; + if (method === 'get') { + return (await axios.get(url, { headers })).data; + } else { + return (await axios.post(url, { headers })).data; + } }, }; },