From 1cdfdc9918acb38284258739d662d27fb8251735 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 3 Jul 2024 11:42:26 +0000 Subject: [PATCH 1/6] test: Fix e2e test race condition by buffering events --- .../test-utils/src/event-proxy-server.ts | 159 +++++++++++------- 1 file changed, 101 insertions(+), 58 deletions(-) diff --git a/dev-packages/test-utils/src/event-proxy-server.ts b/dev-packages/test-utils/src/event-proxy-server.ts index 30bedadc38bb..dcc92b9f6166 100644 --- a/dev-packages/test-utils/src/event-proxy-server.ts +++ b/dev-packages/test-utils/src/event-proxy-server.ts @@ -1,3 +1,5 @@ +/* eslint-disable max-lines */ + import * as fs from 'fs'; import * as http from 'http'; import type { AddressInfo } from 'net'; @@ -30,12 +32,22 @@ interface SentryRequestCallbackData { sentryResponseStatusCode?: number; } +interface EventCallbackListener { + (data: string): void; +} + type OnRequest = ( - eventCallbackListeners: Set<(data: string) => void>, + eventCallbackListeners: Set, proxyRequest: http.IncomingMessage, proxyRequestBody: string, + eventBuffer: BufferedEvent[], ) => Promise<[number, string, Record | undefined]>; +interface BufferedEvent { + timestamp: number; + data: string; +} + /** * Start a generic proxy server. * The `onRequest` callback receives the incoming request and the request body, @@ -51,7 +63,8 @@ export async function startProxyServer( }, onRequest?: OnRequest, ): Promise { - const eventCallbackListeners: Set<(data: string) => void> = new Set(); + const eventBuffer: BufferedEvent[] = []; + const eventCallbackListeners: Set = new Set(); const proxyServer = http.createServer((proxyRequest, proxyResponse) => { const proxyRequestChunks: Uint8Array[] = []; @@ -76,7 +89,9 @@ export async function startProxyServer( const callback: OnRequest = onRequest || - (async (eventCallbackListeners, proxyRequest, proxyRequestBody) => { + (async (eventCallbackListeners, proxyRequest, proxyRequestBody, eventBuffer) => { + eventBuffer.push({ data: proxyRequestBody, timestamp: Date.now() }); + eventCallbackListeners.forEach(listener => { listener(proxyRequestBody); }); @@ -84,7 +99,7 @@ export async function startProxyServer( return [200, '{}', {}]; }); - callback(eventCallbackListeners, proxyRequest, proxyRequestBody) + callback(eventCallbackListeners, proxyRequest, proxyRequestBody, eventBuffer) .then(([statusCode, responseBody, responseHeaders]) => { proxyResponse.writeHead(statusCode, responseHeaders); proxyResponse.write(responseBody, 'utf-8'); @@ -110,12 +125,24 @@ export async function startProxyServer( eventCallbackResponse.statusCode = 200; eventCallbackResponse.setHeader('connection', 'keep-alive'); + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const searchParams = new URL(eventCallbackRequest.url!, 'http://justsomerandombasesothattheurlisparseable.com/') + .searchParams; + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + const listenerTimestamp = Number(searchParams.get('timestamp')!); + const callbackListener = (data: string): void => { eventCallbackResponse.write(data.concat('\n'), 'utf8'); }; eventCallbackListeners.add(callbackListener); + eventBuffer.forEach(bufferedEvent => { + if (bufferedEvent.timestamp > listenerTimestamp) { + callbackListener(bufferedEvent.data); + } + }); + eventCallbackRequest.on('close', () => { eventCallbackListeners.delete(callbackListener); }); @@ -142,7 +169,7 @@ export async function startProxyServer( * option to this server (like this `tunnel: http://localhost:${port option}/`). */ export async function startEventProxyServer(options: EventProxyServerOptions): Promise { - await startProxyServer(options, async (eventCallbackListeners, proxyRequest, proxyRequestBody) => { + await startProxyServer(options, async (eventCallbackListeners, proxyRequest, proxyRequestBody, eventBuffer) => { const envelopeHeader: EnvelopeItem[0] = JSON.parse(proxyRequestBody.split('\n')[0] as string); const shouldForwardEventToSentry = options.forwardToSentry != null ? options.forwardToSentry : true; @@ -199,8 +226,12 @@ export async function startEventProxyServer(options: EventProxyServerOptions): P sentryResponseStatusCode: res.status, }; + const dataString = Buffer.from(JSON.stringify(data)).toString('base64'); + + eventBuffer.push({ data: dataString, timestamp: Date.now() }); + eventCallbackListeners.forEach(listener => { - listener(Buffer.from(JSON.stringify(data)).toString('base64')); + listener(dataString); }); const resHeaders: Record = {}; @@ -221,24 +252,28 @@ export async function waitForPlainRequest( const eventCallbackServerPort = await retrieveCallbackServerPort(proxyServerName); return new Promise((resolve, reject) => { - const request = http.request(`http://localhost:${eventCallbackServerPort}/`, {}, response => { - let eventContents = ''; - - response.on('error', err => { - reject(err); - }); + const request = http.request( + `http://localhost:${eventCallbackServerPort}/?timestamp=${Date.now()}`, + {}, + response => { + let eventContents = ''; + + response.on('error', err => { + reject(err); + }); - response.on('data', (chunk: Buffer) => { - const chunkString = chunk.toString('utf8'); + response.on('data', (chunk: Buffer) => { + const chunkString = chunk.toString('utf8'); - eventContents = eventContents.concat(chunkString); + eventContents = eventContents.concat(chunkString); - if (callback(eventContents)) { - response.destroy(); - return resolve(eventContents); - } - }); - }); + if (callback(eventContents)) { + response.destroy(); + return resolve(eventContents); + } + }); + }, + ); request.end(); }); @@ -247,49 +282,54 @@ export async function waitForPlainRequest( /** Wait for a request to be sent. */ export async function waitForRequest( proxyServerName: string, + timestamp: number, callback: (eventData: SentryRequestCallbackData) => Promise | boolean, ): Promise { const eventCallbackServerPort = await retrieveCallbackServerPort(proxyServerName); return new Promise((resolve, reject) => { - const request = http.request(`http://localhost:${eventCallbackServerPort}/`, {}, response => { - let eventContents = ''; - - response.on('error', err => { - reject(err); - }); + const request = http.request( + `http://localhost:${eventCallbackServerPort}/?timestamp=${timestamp}`, + {}, + response => { + let eventContents = ''; + + response.on('error', err => { + reject(err); + }); - response.on('data', (chunk: Buffer) => { - const chunkString = chunk.toString('utf8'); - chunkString.split('').forEach(char => { - if (char === '\n') { - const eventCallbackData: SentryRequestCallbackData = JSON.parse( - Buffer.from(eventContents, 'base64').toString('utf8'), - ); - const callbackResult = callback(eventCallbackData); - if (typeof callbackResult !== 'boolean') { - callbackResult.then( - match => { - if (match) { - response.destroy(); - resolve(eventCallbackData); - } - }, - err => { - throw err; - }, + response.on('data', (chunk: Buffer) => { + const chunkString = chunk.toString('utf8'); + chunkString.split('').forEach(char => { + if (char === '\n') { + const eventCallbackData: SentryRequestCallbackData = JSON.parse( + Buffer.from(eventContents, 'base64').toString('utf8'), ); - } else if (callbackResult) { - response.destroy(); - resolve(eventCallbackData); + const callbackResult = callback(eventCallbackData); + if (typeof callbackResult !== 'boolean') { + callbackResult.then( + match => { + if (match) { + response.destroy(); + resolve(eventCallbackData); + } + }, + err => { + throw err; + }, + ); + } else if (callbackResult) { + response.destroy(); + resolve(eventCallbackData); + } + eventContents = ''; + } else { + eventContents = eventContents.concat(char); } - eventContents = ''; - } else { - eventContents = eventContents.concat(char); - } + }); }); - }); - }); + }, + ); request.end(); }); @@ -298,10 +338,11 @@ export async function waitForRequest( /** Wait for a specific envelope item to be sent. */ export function waitForEnvelopeItem( proxyServerName: string, + timestamp: number, callback: (envelopeItem: EnvelopeItem) => Promise | boolean, ): Promise { return new Promise((resolve, reject) => { - waitForRequest(proxyServerName, async eventData => { + waitForRequest(proxyServerName, timestamp, async eventData => { const envelopeItems = eventData.envelope[1]; for (const envelopeItem of envelopeItems) { if (await callback(envelopeItem)) { @@ -319,8 +360,9 @@ export function waitForError( proxyServerName: string, callback: (transactionEvent: Event) => Promise | boolean, ): Promise { + const timestamp = Date.now(); return new Promise((resolve, reject) => { - waitForEnvelopeItem(proxyServerName, async envelopeItem => { + waitForEnvelopeItem(proxyServerName, timestamp, async envelopeItem => { const [envelopeItemHeader, envelopeItemBody] = envelopeItem; if (envelopeItemHeader.type === 'event' && (await callback(envelopeItemBody as Event))) { resolve(envelopeItemBody as Event); @@ -336,8 +378,9 @@ export function waitForTransaction( proxyServerName: string, callback: (transactionEvent: Event) => Promise | boolean, ): Promise { + const timestamp = Date.now(); return new Promise((resolve, reject) => { - waitForEnvelopeItem(proxyServerName, async envelopeItem => { + waitForEnvelopeItem(proxyServerName, timestamp, async envelopeItem => { const [envelopeItemHeader, envelopeItemBody] = envelopeItem; if (envelopeItemHeader.type === 'transaction' && (await callback(envelopeItemBody as Event))) { resolve(envelopeItemBody as Event); From 5b8b053b2f8625267fd539f7b85a434470bab614 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 3 Jul 2024 12:04:13 +0000 Subject: [PATCH 2/6] hmm --- .github/workflows/build.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index ba49c52fef94..3df961a6c778 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -100,6 +100,7 @@ jobs: - 'packages/rollup-utils/**' - 'packages/utils/**' - 'packages/types/**' + - 'dev-packages/test-utils/**' browser: &browser - *shared - 'packages/browser/**' From 389b12a13a3402ec7f2397c70389aeb4a135b710 Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 3 Jul 2024 12:04:54 +0000 Subject: [PATCH 3/6] retrigger ci From 831736e0857cba236310eb49b1c74c461020627e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 3 Jul 2024 15:22:02 +0000 Subject: [PATCH 4/6] . --- .../test-applications/react-router-6/tests/transactions.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts index 39e07b89c0ee..421ced591188 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts @@ -56,7 +56,7 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) }); test('sends an INP span', async ({ page }) => { - const inpSpanPromise = waitForEnvelopeItem('react-router-6', item => { + const inpSpanPromise = waitForEnvelopeItem('react-router-6', Date.now(), item => { return item[0].type === 'span'; }); From 25ebcb02fe49a58fdc3f6d5f3b17cfd6e2bdf32e Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Wed, 3 Jul 2024 15:48:17 +0000 Subject: [PATCH 5/6] make optional --- .../react-router-6/tests/transactions.test.ts | 2 +- .../test-utils/src/event-proxy-server.ts | 66 +++++++++++-------- 2 files changed, 40 insertions(+), 28 deletions(-) diff --git a/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts b/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts index 421ced591188..39e07b89c0ee 100644 --- a/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts +++ b/dev-packages/e2e-tests/test-applications/react-router-6/tests/transactions.test.ts @@ -56,7 +56,7 @@ test('sends a navigation transaction with a parameterized URL', async ({ page }) }); test('sends an INP span', async ({ page }) => { - const inpSpanPromise = waitForEnvelopeItem('react-router-6', Date.now(), item => { + const inpSpanPromise = waitForEnvelopeItem('react-router-6', item => { return item[0].type === 'span'; }); diff --git a/dev-packages/test-utils/src/event-proxy-server.ts b/dev-packages/test-utils/src/event-proxy-server.ts index dcc92b9f6166..311ce1e9cac1 100644 --- a/dev-packages/test-utils/src/event-proxy-server.ts +++ b/dev-packages/test-utils/src/event-proxy-server.ts @@ -282,8 +282,8 @@ export async function waitForPlainRequest( /** Wait for a request to be sent. */ export async function waitForRequest( proxyServerName: string, - timestamp: number, callback: (eventData: SentryRequestCallbackData) => Promise | boolean, + timestamp: number = Date.now(), ): Promise { const eventCallbackServerPort = await retrieveCallbackServerPort(proxyServerName); @@ -338,20 +338,24 @@ export async function waitForRequest( /** Wait for a specific envelope item to be sent. */ export function waitForEnvelopeItem( proxyServerName: string, - timestamp: number, callback: (envelopeItem: EnvelopeItem) => Promise | boolean, + timestamp: number = Date.now(), ): Promise { return new Promise((resolve, reject) => { - waitForRequest(proxyServerName, timestamp, async eventData => { - const envelopeItems = eventData.envelope[1]; - for (const envelopeItem of envelopeItems) { - if (await callback(envelopeItem)) { - resolve(envelopeItem); - return true; + waitForRequest( + proxyServerName, + async eventData => { + const envelopeItems = eventData.envelope[1]; + for (const envelopeItem of envelopeItems) { + if (await callback(envelopeItem)) { + resolve(envelopeItem); + return true; + } } - } - return false; - }).catch(reject); + return false; + }, + timestamp, + ).catch(reject); }); } @@ -362,14 +366,18 @@ export function waitForError( ): Promise { const timestamp = Date.now(); return new Promise((resolve, reject) => { - waitForEnvelopeItem(proxyServerName, timestamp, async envelopeItem => { - const [envelopeItemHeader, envelopeItemBody] = envelopeItem; - if (envelopeItemHeader.type === 'event' && (await callback(envelopeItemBody as Event))) { - resolve(envelopeItemBody as Event); - return true; - } - return false; - }).catch(reject); + waitForEnvelopeItem( + proxyServerName, + async envelopeItem => { + const [envelopeItemHeader, envelopeItemBody] = envelopeItem; + if (envelopeItemHeader.type === 'event' && (await callback(envelopeItemBody as Event))) { + resolve(envelopeItemBody as Event); + return true; + } + return false; + }, + timestamp, + ).catch(reject); }); } @@ -380,14 +388,18 @@ export function waitForTransaction( ): Promise { const timestamp = Date.now(); return new Promise((resolve, reject) => { - waitForEnvelopeItem(proxyServerName, timestamp, async envelopeItem => { - const [envelopeItemHeader, envelopeItemBody] = envelopeItem; - if (envelopeItemHeader.type === 'transaction' && (await callback(envelopeItemBody as Event))) { - resolve(envelopeItemBody as Event); - return true; - } - return false; - }).catch(reject); + waitForEnvelopeItem( + proxyServerName, + async envelopeItem => { + const [envelopeItemHeader, envelopeItemBody] = envelopeItem; + if (envelopeItemHeader.type === 'transaction' && (await callback(envelopeItemBody as Event))) { + resolve(envelopeItemBody as Event); + return true; + } + return false; + }, + timestamp, + ).catch(reject); }); } From 5ad28ed568289032a40245a905bedcfefb6ab11a Mon Sep 17 00:00:00 2001 From: Luca Forstner Date: Thu, 4 Jul 2024 09:37:46 +0200 Subject: [PATCH 6/6] Update dev-packages/test-utils/src/event-proxy-server.ts Co-authored-by: Francesco Novy --- dev-packages/test-utils/src/event-proxy-server.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dev-packages/test-utils/src/event-proxy-server.ts b/dev-packages/test-utils/src/event-proxy-server.ts index 311ce1e9cac1..e4eb48f03076 100644 --- a/dev-packages/test-utils/src/event-proxy-server.ts +++ b/dev-packages/test-utils/src/event-proxy-server.ts @@ -138,7 +138,7 @@ export async function startProxyServer( eventCallbackListeners.add(callbackListener); eventBuffer.forEach(bufferedEvent => { - if (bufferedEvent.timestamp > listenerTimestamp) { + if (bufferedEvent.timestamp >= listenerTimestamp) { callbackListener(bufferedEvent.data); } });