From 6e5b79ae08024e18c84b10fc8c3c4df5bce5d9c6 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Wed, 26 Aug 2026 21:58:44 +0530 Subject: [PATCH 1/7] fix(o11y): never let queue-flush instrumentation skip the customer's tests [SDK-7399] Observability events queued during a test are flushed from the internal beforeEach / afterEach hooks in bin/testObservability/cypress/index.js. Those flush sites have no error handling, so an instrumentation failure throws inside a mocha hook. A failing hook makes mocha skip every remaining test in that suite, and those tests are then reported as skipped even though the customer never skipped them. Regression window: up to v1.32.8 the same events were dispatched from Cypress.on(...) listeners -- outside any mocha hook -- and each dispatch was wrapped in .catch(), so an instrumentation failure could not affect the run. v1.33.0 moved the dispatch into these hooks and dropped the error handling. Customers on 1.35.x / 1.36.x see large numbers of tests reported as skipped; the same suite on 1.32.8 is clean. Measured on an 11-test spec whose afterEach issues one failing task, mirroring the flush site: before 1 failing + 10 SKIPPED "Because this error occurred during a `after each` hook we are skipping the remaining tests in the current suite" after 11 passing + 0 skipped Verified end to end on BrowserStack with an 11-test spec that reproduces the customer's shape (cy.session per test, per-test retry overrides, fixture chains in before()): unpatched 1.36.9 -> spec status passed_with_skipped, 985s this change -> spec status passed, 0 skipped, 163s Both observed failure modes are handled: - cy.task / cy.now can throw SYNCHRONOUSLY out of Cypress' runPrivilegedCommand (TypeError: Cannot read properties of null (reading 'get')). A promise .catch() never runs for that, so a real try/catch is required -- confirmed by experiment. - an async rejection, covered by the promise .catch(). cy.now('task', ...) replaces cy.task(...) because cy.task enqueues a Cypress command whose failure surfaces later while the queue drains, failing the hook regardless of any guard around the enqueue call. cy.now executes immediately and returns a promise, which is what v1.32.8 did and is therefore containable. The queue is also cleared before dispatch so a throw cannot replay the same events on the next flush. Co-Authored-By: Claude Opus 5 (1M context) --- bin/testObservability/cypress/index.js | 61 +++++++++++++++++++------- 1 file changed, 46 insertions(+), 15 deletions(-) diff --git a/bin/testObservability/cypress/index.js b/bin/testObservability/cypress/index.js index 7c11ae10..7e1c89c3 100644 --- a/bin/testObservability/cypress/index.js +++ b/bin/testObservability/cypress/index.js @@ -339,6 +339,50 @@ Cypress.Commands.add('fatal', (message, file) => { }); }); +/* + * [SDK-7399] Drain eventsQueue without ever being able to fail the customer's suite. + * + * These flush sites run inside mocha `beforeEach`/`afterEach`. Up to v1.32.8 the same + * events were dispatched from `Cypress.on(...)` listeners — i.e. OUTSIDE any mocha hook — + * and every dispatch was additionally wrapped in `.catch()`, so an instrumentation + * failure could not affect the run. v1.33.0 moved the dispatch INTO these hooks and + * dropped all error handling, which makes instrumentation errors fatal to the customer: + * a throw inside a hook fails that hook, and mocha then SKIPS EVERY REMAINING TEST in + * the suite. That is the reported symptom — tests reported as skipped that the customer + * never skipped. + * + * Two independent guards are required, because both failure modes were observed: + * 1. SYNCHRONOUS throw — `cy.task`/`cy.now` can throw straight out of the call + * (`TypeError: Cannot read properties of null (reading 'get')` raised inside + * Cypress' own `runPrivilegedCommand`). A promise `.catch()` never runs for this, + * so a real try/catch is needed. + * 2. ASYNCHRONOUS rejection — handled by the promise `.catch()`. + * + * `cy.now('task', ...)` is used rather than `cy.task(...)`: `cy.task` enqueues a Cypress + * command, so a failure surfaces later while the queue drains and fails the hook no + * matter what guard wraps the enqueue call. `cy.now` executes immediately and returns a + * promise, which is exactly what v1.32.8 did and is therefore containable here. + */ +const flushEventsQueue = () => { + try { + const queued = eventsQueue; + eventsQueue = []; + queued.forEach(event => { + try { + const payload = sanitizeForTask(event.data); + if (payload === null) return; + const result = cy.now('task', event.task, payload, event.options); + if (result && typeof result.catch === 'function') result.catch(() => {}); + } catch (e) { + /* one bad event must not stop the remaining events, and must not fail the hook */ + } + }); + } catch (e) { + /* instrumentation must never break the customer's test run */ + eventsQueue = []; + } +}; + beforeEach(() => { /* browserstack internal helper hook */ @@ -346,13 +390,7 @@ beforeEach(() => { return; } - if (eventsQueue.length > 0) { - eventsQueue.forEach(event => { - const payload = sanitizeForTask(event.data); - if (payload !== null) cy.task(event.task, payload, event.options); - }); - } - eventsQueue = []; + flushEventsQueue(); testRunStarted = true; }); @@ -362,13 +400,6 @@ afterEach(function() { return; } - if (eventsQueue.length > 0) { - eventsQueue.forEach(event => { - const payload = sanitizeForTask(event.data); - if (payload !== null) cy.task(event.task, payload, event.options); - }); - } - - eventsQueue = []; + flushEventsQueue(); testRunStarted = false; }); From 82cf3f92a86aad80d9743468617782388dd6e784 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 27 Aug 2026 13:57:29 +0530 Subject: [PATCH 2/7] fix(o11y): log every suppressed flush failure instead of dropping it silently Review follow-up. The three suppression points in flushEventsQueue (async promise rejection, per-event throw, whole-flush throw) protected the customer's run but left no trace at all, so a dropped observability event would only surface later as data quietly missing from the dashboard -- trading a loud failure for a silent one. Each now routes through warnFlushFailure(), which names the stage and the failing task. console.warn is used deliberately rather than browserStackLog/cy.task: routing a diagnostic through another Cypress command would reintroduce the enqueue-time failure this change exists to contain. Same convention already used for the equivalent case in bin/accessibility-automation/cypress/index.js ("suppressed afterEach error"). warnFlushFailure is itself wrapped so logging can never throw. Re-verified the guarantee is unchanged: 11-test spec with a failing dispatch in afterEach -> 11 passing, 0 skipped. Co-Authored-By: Claude Opus 5 (1M context) --- bin/testObservability/cypress/index.js | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/bin/testObservability/cypress/index.js b/bin/testObservability/cypress/index.js index 7e1c89c3..2034e4b2 100644 --- a/bin/testObservability/cypress/index.js +++ b/bin/testObservability/cypress/index.js @@ -363,6 +363,19 @@ Cypress.Commands.add('fatal', (message, file) => { * matter what guard wraps the enqueue call. `cy.now` executes immediately and returns a * promise, which is exactly what v1.32.8 did and is therefore containable here. */ +/* + * Every suppression below must leave a trace. A dropped event is invisible to the + * customer's run by design, but it must not be invisible to us — otherwise a future + * flush failure only shows up as data quietly missing from the dashboard. `console.warn` + * is used deliberately rather than `browserStackLog`/`cy.task`: routing a diagnostic + * through another Cypress command would reintroduce exactly the failure being contained. + */ +const warnFlushFailure = (stage, err) => { + try { + console.warn(`BrowserStack Test Observability: suppressed ${stage} error, event(s) dropped: ${err && err.message ? err.message : err}`); + } catch (e) { /* logging must never throw either */ } +}; + const flushEventsQueue = () => { try { const queued = eventsQueue; @@ -372,13 +385,17 @@ const flushEventsQueue = () => { const payload = sanitizeForTask(event.data); if (payload === null) return; const result = cy.now('task', event.task, payload, event.options); - if (result && typeof result.catch === 'function') result.catch(() => {}); + if (result && typeof result.catch === 'function') { + result.catch(err => warnFlushFailure(`async dispatch of '${event.task}'`, err)); + } } catch (e) { /* one bad event must not stop the remaining events, and must not fail the hook */ + warnFlushFailure(`dispatch of '${event.task}'`, e); } }); } catch (e) { /* instrumentation must never break the customer's test run */ + warnFlushFailure('queue flush', e); eventsQueue = []; } }; From 937f2d94463997fa48900e20906de6ff7cfaa95d Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 27 Aug 2026 14:01:38 +0530 Subject: [PATCH 3/7] chore(o11y): trim the flush-guard comments to the non-obvious rationale Comments outnumbered code roughly 2:1. Kept only what stops the boundary being undone by a later refactor -- why a throw here skips the rest of the spec, why cy.now rather than cy.task, why try/catch as well as .catch, and why console.warn rather than cy.task. No logic change (33 -> 12 comment lines). Co-Authored-By: Claude Opus 5 (1M context) --- bin/testObservability/cypress/index.js | 49 ++++++++------------------ 1 file changed, 14 insertions(+), 35 deletions(-) diff --git a/bin/testObservability/cypress/index.js b/bin/testObservability/cypress/index.js index 2034e4b2..71bf868b 100644 --- a/bin/testObservability/cypress/index.js +++ b/bin/testObservability/cypress/index.js @@ -339,47 +339,28 @@ Cypress.Commands.add('fatal', (message, file) => { }); }); -/* - * [SDK-7399] Drain eventsQueue without ever being able to fail the customer's suite. - * - * These flush sites run inside mocha `beforeEach`/`afterEach`. Up to v1.32.8 the same - * events were dispatched from `Cypress.on(...)` listeners — i.e. OUTSIDE any mocha hook — - * and every dispatch was additionally wrapped in `.catch()`, so an instrumentation - * failure could not affect the run. v1.33.0 moved the dispatch INTO these hooks and - * dropped all error handling, which makes instrumentation errors fatal to the customer: - * a throw inside a hook fails that hook, and mocha then SKIPS EVERY REMAINING TEST in - * the suite. That is the reported symptom — tests reported as skipped that the customer - * never skipped. - * - * Two independent guards are required, because both failure modes were observed: - * 1. SYNCHRONOUS throw — `cy.task`/`cy.now` can throw straight out of the call - * (`TypeError: Cannot read properties of null (reading 'get')` raised inside - * Cypress' own `runPrivilegedCommand`). A promise `.catch()` never runs for this, - * so a real try/catch is needed. - * 2. ASYNCHRONOUS rejection — handled by the promise `.catch()`. - * - * `cy.now('task', ...)` is used rather than `cy.task(...)`: `cy.task` enqueues a Cypress - * command, so a failure surfaces later while the queue drains and fails the hook no - * matter what guard wraps the enqueue call. `cy.now` executes immediately and returns a - * promise, which is exactly what v1.32.8 did and is therefore containable here. - */ -/* - * Every suppression below must leave a trace. A dropped event is invisible to the - * customer's run by design, but it must not be invisible to us — otherwise a future - * flush failure only shows up as data quietly missing from the dashboard. `console.warn` - * is used deliberately rather than `browserStackLog`/`cy.task`: routing a diagnostic - * through another Cypress command would reintroduce exactly the failure being contained. - */ +/* console.warn, not browserStackLog/cy.task — routing a diagnostic through another + * Cypress command would reintroduce the failure this boundary contains. [SDK-7399] */ const warnFlushFailure = (stage, err) => { try { console.warn(`BrowserStack Test Observability: suppressed ${stage} error, event(s) dropped: ${err && err.message ? err.message : err}`); } catch (e) { /* logging must never throw either */ } }; +/* + * [SDK-7399] These flush sites run inside mocha beforeEach/afterEach, so a throw here + * fails the hook and mocha then SKIPS every remaining test in the spec. Before v1.33.0 + * the same events were dispatched from Cypress.on(...) listeners — outside any hook, + * each wrapped in .catch() — so a failure was harmless. Keep this boundary intact: + * - cy.now, not cy.task: cy.task enqueues, so its failure surfaces later during queue + * drain and fails the hook regardless of any guard here. cy.now runs immediately. + * - try/catch AND .catch: cy.now can throw synchronously out of Cypress' + * runPrivilegedCommand, which a promise .catch() never sees. + */ const flushEventsQueue = () => { try { const queued = eventsQueue; - eventsQueue = []; + eventsQueue = []; /* cleared before dispatch so a throw cannot replay these events */ queued.forEach(event => { try { const payload = sanitizeForTask(event.data); @@ -389,12 +370,10 @@ const flushEventsQueue = () => { result.catch(err => warnFlushFailure(`async dispatch of '${event.task}'`, err)); } } catch (e) { - /* one bad event must not stop the remaining events, and must not fail the hook */ - warnFlushFailure(`dispatch of '${event.task}'`, e); + warnFlushFailure(`dispatch of '${event.task}'`, e); /* skip one event, not the rest */ } }); } catch (e) { - /* instrumentation must never break the customer's test run */ warnFlushFailure('queue flush', e); eventsQueue = []; } From a0ef0beeae03b07c0fb4be3a4912ca8226ea4561 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 27 Aug 2026 19:20:13 +0530 Subject: [PATCH 4/7] fix(o11y): prevent oversized cy.task payloads instead of switching dispatch [SDK-7399] Replaces the earlier cy.now approach, which was wrong: cy.now('task', ...) throws on Cypress 14 in every context (test body, mocha hook and Cypress.on listener), verified on a remote Windows terminal, so it stopped the skipping only by never delivering anything. Browser-side telemetry silently disappeared from the dashboard. Dispatch therefore stays on cy.task, which does deliver. Since cy.task enqueues, its failure surfaces after the enqueue call returns and cannot be caught at the call site -- so the protection is to never build a payload that fails. Measured on a remote Windows terminal, single event per afterEach: 64KB pass 128KB pass 256KB pass 512KB pass 768KB pass 1MB FAIL 8MB FAIL Event count is not a factor: 10, 100 and 1000 small events all pass. The limit is a hard ceiling near 1MB per cy.task payload. sanitizeForTask now caps the serialized payload at 128KB: individual strings longer than 8KB are truncated first (command args are the realistic source of bulk), and the event is skipped only if it is still too large. Skips are logged rather than silent. Co-Authored-By: Claude Opus 5 (1M context) --- bin/testObservability/cypress/index.js | 62 ++++++++++++++++++++------ 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/bin/testObservability/cypress/index.js b/bin/testObservability/cypress/index.js index 71bf868b..c1aa448c 100644 --- a/bin/testObservability/cypress/index.js +++ b/bin/testObservability/cypress/index.js @@ -32,9 +32,43 @@ const getCircularReplacer = () => { * the Node o11y handler expects a structured event payload, not an error stub. Skipping keeps * graceful degradation total: no crash, and no malformed event reaches the collector. */ +/* + * [SDK-7399] An oversized cy.task payload fails the command, and because the flush runs + * inside a mocha hook that failure skips every remaining test in the spec. Measured on a + * remote Windows terminal: a single 64KB payload succeeds, 1MB and 8MB fail; event COUNT + * is not the problem (1000 small events succeed). Command args are the realistic source + * of bulk, so cap individual strings first and only drop the event if it is still too + * large. Preventing the oversized dispatch is what keeps the customer's suite intact — + * containment alone cannot, since the failure surfaces after the enqueue call returns. + */ +const MAX_TASK_PAYLOAD_CHARS = 128 * 1024; +const MAX_STRING_CHARS = 8 * 1024; +const TRUNCATION_MARKER = '…[browserstack: truncated]'; + +const getTruncatingReplacer = () => { + const seen = new WeakSet(); + return (key, value) => { + if (typeof value === 'string' && value.length > MAX_STRING_CHARS) { + return value.slice(0, MAX_STRING_CHARS) + TRUNCATION_MARKER; + } + if (typeof value === 'object' && value !== null) { + if (seen.has(value)) return '[Circular]'; + seen.add(value); + } + return value; + }; +}; + +/* Returns a JSON-safe plain object small enough to ship, or `null` to skip the event. */ const sanitizeForTask = (data) => { try { - return JSON.parse(JSON.stringify(data, getCircularReplacer())); + let json = JSON.stringify(data, getCircularReplacer()); + if (json === undefined) return null; + if (json.length > MAX_TASK_PAYLOAD_CHARS) { + json = JSON.stringify(data, getTruncatingReplacer()); + if (json === undefined || json.length > MAX_TASK_PAYLOAD_CHARS) return null; + } + return JSON.parse(json); } catch (e) { return null; } @@ -348,14 +382,15 @@ const warnFlushFailure = (stage, err) => { }; /* - * [SDK-7399] These flush sites run inside mocha beforeEach/afterEach, so a throw here - * fails the hook and mocha then SKIPS every remaining test in the spec. Before v1.33.0 - * the same events were dispatched from Cypress.on(...) listeners — outside any hook, - * each wrapped in .catch() — so a failure was harmless. Keep this boundary intact: - * - cy.now, not cy.task: cy.task enqueues, so its failure surfaces later during queue - * drain and fails the hook regardless of any guard here. cy.now runs immediately. - * - try/catch AND .catch: cy.now can throw synchronously out of Cypress' - * runPrivilegedCommand, which a promise .catch() never sees. + * [SDK-7399] These flush sites run inside mocha beforeEach/afterEach, so a failing + * dispatch here fails the hook and mocha then SKIPS every remaining test in the spec. + * Dispatch stays on cy.task: it is the only form that actually delivers (cy.now('task') + * throws on Cypress 14 in every context — test body, hook and listener — so switching to + * it silently drops all browser-side telemetry). Because cy.task enqueues, its failure + * surfaces after this function returns and cannot be caught here; the protection is + * therefore to never build a payload that fails — see sanitizeForTask's size cap. The + * try/catch below remains as a backstop for anything raised synchronously while building + * or enqueuing an event. */ const flushEventsQueue = () => { try { @@ -364,11 +399,12 @@ const flushEventsQueue = () => { queued.forEach(event => { try { const payload = sanitizeForTask(event.data); - if (payload === null) return; - const result = cy.now('task', event.task, payload, event.options); - if (result && typeof result.catch === 'function') { - result.catch(err => warnFlushFailure(`async dispatch of '${event.task}'`, err)); + if (payload === null) { + warnFlushFailure(`oversized or unserializable payload for '${event.task}'`, + new Error('event skipped')); + return; } + cy.task(event.task, payload, event.options); } catch (e) { warnFlushFailure(`dispatch of '${event.task}'`, e); /* skip one event, not the rest */ } From f64d61d34ef8c874d8cbba393ba54e28df1c909f Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 27 Aug 2026 19:59:18 +0530 Subject: [PATCH 5/7] fix(o11y): batch the event flush into one cy.task call [SDK-7399] The real cause of the reported skipping is a TIMEOUT, not an exception. Nothing throws: build-info on a reproducing build reports failed:0, success:6, ignored:27 with all three sessions status=error at ~985s against spec_timeout 900000ms. The specs are killed at spec_timeout and every test that has not run yet is reported as skipped. Each cy.task round-trip costs roughly 0.8s on a remote terminal. Measured there, 6 tests per spec, N events per afterEach: N=1 -> 6 calls -> 113s N=100 -> 600 calls -> 581s N=10 -> 60 calls -> 114s N=1000 -> 6000 calls -> session killed A command-heavy test queues hundreds of events, so the old one-cy.task-per-event flush spent minutes in the hook and blew the spec budget. Locally the same dispatch is effectively free, which is why every local run passed. Batching verified before writing this: the same 600 events sent as ONE cy.task call completed in 109s versus 581s as 600 calls -- i.e. back to baseline. Changes: - plugin: new test_observability_batch task takes an array and fans out to the same IPC events; the four individual tasks stay registered for backward compatibility. - cypress: the flush builds one batch per drain, split at 512KB so each call stays under the ~1MB per-payload ceiling also measured on the remote (768KB passes, 1MB fails). - shouldSkipCommand filters test_observability_batch, otherwise each batch dispatch would be captured as a command event and refill the queue. - sanitizeForTask keeps the 128KB payload cap with 8KB string truncation. This supersedes the earlier cy.now approach, which stopped the skipping only by never delivering anything: cy.now('task') throws on Cypress 14 in every context, so all browser-side telemetry silently disappeared from the dashboard. Co-Authored-By: Claude Opus 5 (1M context) --- bin/testObservability/cypress/index.js | 39 ++++++++++++++++++++++---- bin/testObservability/plugin/index.js | 30 ++++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/bin/testObservability/cypress/index.js b/bin/testObservability/cypress/index.js index c1aa448c..c996f32a 100644 --- a/bin/testObservability/cypress/index.js +++ b/bin/testObservability/cypress/index.js @@ -31,8 +31,7 @@ const getCircularReplacer = () => { * `null` is a "skip this event" sentinel — callers must NOT forward it to cy.task, because * the Node o11y handler expects a structured event payload, not an error stub. Skipping keeps * graceful degradation total: no crash, and no malformed event reaches the collector. - */ -/* + * * [SDK-7399] An oversized cy.task payload fails the command, and because the flush runs * inside a mocha hook that failure skips every remaining test in the spec. Measured on a * remote Windows terminal: a single 64KB payload succeeds, 1MB and 8MB fail; event COUNT @@ -84,7 +83,9 @@ const shouldSkipCommand = (command) => { if (!Cypress.env('BROWSERSTACK_O11Y_LOGS')) { return true; } - return command.attributes.name == 'log' || (command.attributes.name == 'task' && (['test_observability_platform_details', 'test_observability_step', 'test_observability_command', 'browserstack_log', 'test_observability_log'].some(event => command.attributes.args.includes(event)))); + /* test_observability_batch must be filtered here too, or each batch dispatch would + * itself be captured as a command event and refill the queue. [SDK-7399] */ + return command.attributes.name == 'log' || (command.attributes.name == 'task' && (['test_observability_platform_details', 'test_observability_step', 'test_observability_command', 'test_observability_batch', 'browserstack_log', 'test_observability_log'].some(event => command.attributes.args.includes(event)))); } Cypress.on('log:changed', (attrs) => { @@ -384,7 +385,7 @@ const warnFlushFailure = (stage, err) => { /* * [SDK-7399] These flush sites run inside mocha beforeEach/afterEach, so a failing * dispatch here fails the hook and mocha then SKIPS every remaining test in the spec. - * Dispatch stays on cy.task: it is the only form that actually delivers (cy.now('task') + * Dispatch stays on cy.task (cy.now('task') * throws on Cypress 14 in every context — test body, hook and listener — so switching to * it silently drops all browser-side telemetry). Because cy.task enqueues, its failure * surfaces after this function returns and cannot be caught here; the protection is @@ -392,10 +393,31 @@ const warnFlushFailure = (stage, err) => { * try/catch below remains as a backstop for anything raised synchronously while building * or enqueuing an event. */ +/* Keep each batch comfortably under the ~1MB per-cy.task ceiling measured on a remote + * terminal (768KB succeeds, 1MB fails), so a large flush is split rather than dropped. */ +const MAX_BATCH_CHARS = 512 * 1024; + const flushEventsQueue = () => { try { const queued = eventsQueue; eventsQueue = []; /* cleared before dispatch so a throw cannot replay these events */ + if (queued.length === 0) return; + + let batch = []; + let batchChars = 0; + + const sendBatch = () => { + if (batch.length === 0) return; + const toSend = batch; + batch = []; + batchChars = 0; + try { + cy.task('test_observability_batch', toSend, { log: false }); + } catch (e) { + warnFlushFailure(`batch dispatch of ${toSend.length} event(s)`, e); + } + }; + queued.forEach(event => { try { const payload = sanitizeForTask(event.data); @@ -404,11 +426,16 @@ const flushEventsQueue = () => { new Error('event skipped')); return; } - cy.task(event.task, payload, event.options); + const size = JSON.stringify(payload).length; + if (batchChars + size > MAX_BATCH_CHARS) sendBatch(); + batch.push({ task: event.task, data: payload }); + batchChars += size; } catch (e) { - warnFlushFailure(`dispatch of '${event.task}'`, e); /* skip one event, not the rest */ + warnFlushFailure(`preparing '${event.task}'`, e); /* skip one event, not the rest */ } }); + + sendBatch(); } catch (e) { warnFlushFailure('queue flush', e); eventsQueue = []; diff --git a/bin/testObservability/plugin/index.js b/bin/testObservability/plugin/index.js index d32ade0d..f13d012b 100644 --- a/bin/testObservability/plugin/index.js +++ b/bin/testObservability/plugin/index.js @@ -11,6 +11,13 @@ const browserstackTestObservabilityPlugin = (on, config, callbacks) => { connectIPCClient(config); + const IPC_EVENT_FOR_TASK = { + test_observability_log: IPC_EVENTS.LOG, + test_observability_command: IPC_EVENTS.COMMAND, + test_observability_platform_details: IPC_EVENTS.PLATFORM_DETAILS, + test_observability_step: IPC_EVENTS.CUCUMBER, + }; + on('task', { test_observability_log(log) { ipc.of.browserstackTestObservability.emit(IPC_EVENTS.LOG, log); @@ -27,6 +34,29 @@ const browserstackTestObservabilityPlugin = (on, config, callbacks) => { test_observability_step(log) { ipc.of.browserstackTestObservability.emit(IPC_EVENTS.CUCUMBER, log); return null; + }, + /* + * [SDK-7399] Accepts a whole flush as ONE task so the browser side issues one + * Cypress command per flush instead of one per event. Each cy.task round-trip costs + * roughly 0.8s on a remote terminal, so a command-heavy test used to spend minutes + * in its afterEach and the spec was killed at spec_timeout, reporting every test that + * had not run yet as skipped. Measured: 600 events as 600 calls = 581s; the same 600 + * events as 1 call = 109s, i.e. baseline. + * Fans out to exactly the same IPC events as the individual tasks above, which stay + * registered for backward compatibility. + */ + test_observability_batch(events) { + if (!Array.isArray(events)) return null; + events.forEach((event) => { + try { + const ipcEvent = event && IPC_EVENT_FOR_TASK[event.task]; + if (!ipcEvent) return; + ipc.of.browserstackTestObservability.emit(ipcEvent, event.data); + } catch (e) { + /* one malformed event must not drop the rest of the batch */ + } + }); + return null; } }); From abb89b63c2c82db3f13eabf226ae50cba50ef6a8 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 27 Aug 2026 20:37:40 +0530 Subject: [PATCH 6/7] fix(o11y): drop the string truncation, keep only what the batching needs [SDK-7399] The 128KB per-event cap and 8KB string truncation were written for an earlier, wrong theory (that oversized payloads were the cause). Batching is what fixes the timeout, so the truncation fixed nothing and would have changed behaviour for customers who work fine today: any command arg or log string over 8KB would have started arriving truncated. Removed. Nothing under 512KB is altered any more. Kept: the 512KB batch split, which is required -- a batch of many events can otherwise cross the ~1MB per-cy.task ceiling measured on a remote terminal (768KB passes, 1MB fails). Added: a single event larger than 512KB is dropped with a log line instead of being sent. That is not a fidelity regression -- before batching such an event was dispatched on its own and would have failed the command anyway, taking the spec with it. Co-Authored-By: Claude Opus 5 (1M context) --- bin/testObservability/cypress/index.js | 43 ++++++-------------------- 1 file changed, 10 insertions(+), 33 deletions(-) diff --git a/bin/testObservability/cypress/index.js b/bin/testObservability/cypress/index.js index c996f32a..63b8b90d 100644 --- a/bin/testObservability/cypress/index.js +++ b/bin/testObservability/cypress/index.js @@ -32,42 +32,10 @@ const getCircularReplacer = () => { * the Node o11y handler expects a structured event payload, not an error stub. Skipping keeps * graceful degradation total: no crash, and no malformed event reaches the collector. * - * [SDK-7399] An oversized cy.task payload fails the command, and because the flush runs - * inside a mocha hook that failure skips every remaining test in the spec. Measured on a - * remote Windows terminal: a single 64KB payload succeeds, 1MB and 8MB fail; event COUNT - * is not the problem (1000 small events succeed). Command args are the realistic source - * of bulk, so cap individual strings first and only drop the event if it is still too - * large. Preventing the oversized dispatch is what keeps the customer's suite intact — - * containment alone cannot, since the failure surfaces after the enqueue call returns. */ -const MAX_TASK_PAYLOAD_CHARS = 128 * 1024; -const MAX_STRING_CHARS = 8 * 1024; -const TRUNCATION_MARKER = '…[browserstack: truncated]'; - -const getTruncatingReplacer = () => { - const seen = new WeakSet(); - return (key, value) => { - if (typeof value === 'string' && value.length > MAX_STRING_CHARS) { - return value.slice(0, MAX_STRING_CHARS) + TRUNCATION_MARKER; - } - if (typeof value === 'object' && value !== null) { - if (seen.has(value)) return '[Circular]'; - seen.add(value); - } - return value; - }; -}; - -/* Returns a JSON-safe plain object small enough to ship, or `null` to skip the event. */ const sanitizeForTask = (data) => { try { - let json = JSON.stringify(data, getCircularReplacer()); - if (json === undefined) return null; - if (json.length > MAX_TASK_PAYLOAD_CHARS) { - json = JSON.stringify(data, getTruncatingReplacer()); - if (json === undefined || json.length > MAX_TASK_PAYLOAD_CHARS) return null; - } - return JSON.parse(json); + return JSON.parse(JSON.stringify(data, getCircularReplacer())); } catch (e) { return null; } @@ -427,6 +395,15 @@ const flushEventsQueue = () => { return; } const size = JSON.stringify(payload).length; + if (size > MAX_BATCH_CHARS) { + /* A single event this large cannot be sent under the ~1MB per-cy.task ceiling + * measured on a remote terminal (768KB passes, 1MB fails). Dropping it is not a + * fidelity regression: before batching it was dispatched alone and would have + * failed the command anyway. Nothing smaller is altered or truncated. */ + warnFlushFailure(`event too large to send for '${event.task}' (${size} chars)`, + new Error('event skipped')); + return; + } if (batchChars + size > MAX_BATCH_CHARS) sendBatch(); batch.push({ task: event.task, data: payload }); batchChars += size; From 33a92327f2e3e3a3c5b1b2c737ac9520f4124897 Mon Sep 17 00:00:00 2001 From: Shivam Kumar Date: Thu, 27 Aug 2026 21:23:39 +0530 Subject: [PATCH 7/7] docs(o11y): correct the flush comments to match the shipped fix [SDK-7399] The comment above flushEventsQueue still described the earlier payload-cap approach and pointed at "sanitizeForTask's size cap", which no longer exists after abb89b6 removed it. It also never stated batching as the mechanism, which is the actual fix. Rewritten to state the measured cause (~0.8s per cy.task round-trip, N=100 -> 581s, N=1000 -> session killed at spec_timeout, failed:0 so nothing throws) and the measured remedy (600 events as one call: 109s). Also notes why dispatch stays on cy.task, and that the try/catch is only a backstop since a cy.task failure surfaces after the enqueue call. Corrected one log message: sanitizeForTask returning null now means unserializable only, not oversized. Removed an orphan comment line. No functional change. Co-Authored-By: Claude Opus 5 (1M context) --- bin/testObservability/cypress/index.js | 36 ++++++++++++++++---------- 1 file changed, 23 insertions(+), 13 deletions(-) diff --git a/bin/testObservability/cypress/index.js b/bin/testObservability/cypress/index.js index 63b8b90d..1227c38b 100644 --- a/bin/testObservability/cypress/index.js +++ b/bin/testObservability/cypress/index.js @@ -31,7 +31,6 @@ const getCircularReplacer = () => { * `null` is a "skip this event" sentinel — callers must NOT forward it to cy.task, because * the Node o11y handler expects a structured event payload, not an error stub. Skipping keeps * graceful degradation total: no crash, and no malformed event reaches the collector. - * */ const sanitizeForTask = (data) => { try { @@ -351,18 +350,29 @@ const warnFlushFailure = (stage, err) => { }; /* - * [SDK-7399] These flush sites run inside mocha beforeEach/afterEach, so a failing - * dispatch here fails the hook and mocha then SKIPS every remaining test in the spec. - * Dispatch stays on cy.task (cy.now('task') - * throws on Cypress 14 in every context — test body, hook and listener — so switching to - * it silently drops all browser-side telemetry). Because cy.task enqueues, its failure - * surfaces after this function returns and cannot be caught here; the protection is - * therefore to never build a payload that fails — see sanitizeForTask's size cap. The - * try/catch below remains as a backstop for anything raised synchronously while building - * or enqueuing an event. + * [SDK-7399] Send the whole drain as ONE cy.task instead of one cy.task per event. + * + * Each cy.task round-trip costs roughly 0.8s on a remote terminal. Measured there, with + * N events queued per afterEach: N=10 -> 114s, N=100 -> 581s, N=1000 -> the session was + * killed. A command-heavy test queues hundreds of events, so the old per-event flush ran + * for minutes inside the hook, the spec exceeded spec_timeout, and every test that had not + * run yet was reported as SKIPPED. Nothing throws in that failure — build-info on a + * reproducing build shows failed:0 with the sessions killed at the timeout. Locally the + * same dispatch is effectively free, which is why local runs never reproduced it. + * + * Batching is what fixes it: the same 600 events sent as one call took 109s versus 581s. + * + * Dispatch deliberately stays on cy.task. cy.now('task', ...) throws on Cypress 14 in + * every context — test body, hook and listener — so using it stops the skipping only by + * never delivering anything, which silently empties the dashboard. + * + * The try/catch here is a backstop for anything raised synchronously while building or + * enqueuing. It cannot catch a cy.task failure, which surfaces later while the command + * queue drains — hence fixing the cost rather than trying to contain the symptom. */ -/* Keep each batch comfortably under the ~1MB per-cy.task ceiling measured on a remote - * terminal (768KB succeeds, 1MB fails), so a large flush is split rather than dropped. */ + +/* Split each batch under the ~1MB per-cy.task ceiling measured on a remote terminal + * (768KB succeeds, 1MB fails), so a large flush is split rather than lost. */ const MAX_BATCH_CHARS = 512 * 1024; const flushEventsQueue = () => { @@ -390,7 +400,7 @@ const flushEventsQueue = () => { try { const payload = sanitizeForTask(event.data); if (payload === null) { - warnFlushFailure(`oversized or unserializable payload for '${event.task}'`, + warnFlushFailure(`unserializable payload for '${event.task}'`, new Error('event skipped')); return; }