Skip to content

fix(observability): prevent "Converting circular structure to JSON" crash from circular cy.task payloads [SDK-6016] #1118

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.

Sign up for GitHub

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

Open
harshit-browserstack wants to merge 1 commit into
base: master
Choose a base branch
from
+31 −3
Open

fix(observability): prevent "Converting circular structure to JSON" crash from circular cy.task payloads [SDK-6016] #1118

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions bin/testObservability/cypress/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,34 @@ const STEP_KEYWORDS = ['given', 'when', 'then', 'and', 'but', '*'];
let eventsQueue = [];
let testRunStarted = false;

/*
* Command args (command.attributes.args) and cy.log items are captured raw and can hold
* circular Cypress runtime objects (e.g. a config-like object whose `renderOptions.host`
* points back to itself). cy.task() JSON-serializes its payload to ship it from the browser
* to the Node plugin process, so a circular arg makes Cypress throw
* "Converting circular structure to JSON" and aborts the run. Decycle the payload before
* handing it to cy.task so o11y instrumentation can never break the customer's tests. [SDK-6016]
*/
const getCircularReplacer = () => {
Copy link
Copy Markdown
Collaborator

@xxshubhamxx xxshubhamxx Jun 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Suggestion — [CODE-QUALITY] getCircularReplacer allocates a new WeakSet on every call

Creating a new WeakSet per JSON.stringify invocation is correct for cycle detection — each call needs its own independent seen-set to track cycles within that specific value graph. There is no functional issue here.

If command:enqueued is ever profiled as a hot path (high-frequency commands in large test suites), the per-call WeakSet allocation may be worth revisiting. No change required unless profiling shows it as a hotspot.

const seen = new WeakSet();
return (key, value) => {
if (typeof value === 'object' && value !== null) {
if (seen.has(value)) return '[Circular]';
Copy link
Copy Markdown
Collaborator

@xxshubhamxx xxshubhamxx Jun 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Suggestion — [OBSERVABILITY-IMPACT] [Circular] placeholder will appear on the dashboard as a test argument value

When a circular reference is replaced with the string "[Circular]", the Test Observability dashboard will display it inline as if it were the actual test argument value (e.g. a command log entry might read mount [Circular] instead of something meaningful).

This is the correct trade-off — better a [Circular] placeholder than a crashed run — but it may surprise users seeing [Circular] in their command logs for the first time.

Consider mentioning this in the PR/release notes so users are aware that self-referential config objects (like @cypress/vue mount options) will appear as [Circular] in their o11y command logs rather than being omitted entirely.

seen.add(value);
}
return value;
};
};

const sanitizeForTask = (data) => {
try {
return JSON.parse(JSON.stringify(data, getCircularReplacer()));
} catch (e) {
/* Never let serialization of o11y data break the user's test run (graceful degradation). */
return { serializationError: e && e.message ? e.message : String(e) };
Copy link
Copy Markdown
Collaborator

@xxshubhamxx xxshubhamxx Jun 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Warning — [OBSERVABILITY-IMPACT] sanitizeForTask error fallback sends a malformed event payload

Problem

On the catch path, sanitizeForTask returns { serializationError: "..." } and the caller proceeds to send it via cy.task(event.task, { serializationError: "..." }, ...). The Node plugin's o11y task handler expects a structured event payload (fields like testUuid, level, message, timestamp, etc.). Receiving { serializationError: "..." } instead will either:

  1. Silently produce a malformed event on the Test Observability dashboard (missing test association, missing command name), or
  2. Trigger a handler-side error that the Node plugin may not catch, leading to a broken o11y session.

In both cases, the user gets silent data loss or a noisy error in their o11y dashboard from the very path that is supposed to degrade gracefully.

Suggested Fix

Skip the cy.task call entirely when serialization fails. Return null from sanitizeForTask as a "skip this event" sentinel and guard the call sites:

const sanitizeForTask = (data) => {
  try {
    return JSON.parse(JSON.stringify(data, getCircularReplacer()));
  } catch (e) {
    /* Never let serialization of o11y data break the user's test run. */
    browserStackLog(`[o11y] Failed to serialize task payload: ${e && e.message ? e.message : String(e)}`);
    return null; // signal: skip this event
  }
};

Then guard each call site:

// beforeEach / afterEach flush (lines 340, 355)
eventsQueue.forEach(event => {
  const payload = sanitizeForTask(event.data);
  if (payload !== null) cy.task(event.task, payload, event.options);
});

This preserves complete graceful degradation (no crash), logs a debug line visible via BROWSERSTACK_LOGS, and prevents any malformed payload from reaching the collector.

Confidence: 🟢 Grounded in the Test Observability event payload schema; the { serializationError } shape does not match any expected event type the Node plugin handler accepts.

}
};

const browserStackLog = (message) => {

if (!Cypress.env('BROWSERSTACK_LOGS')) return;
Expand Down Expand Up @@ -208,7 +236,7 @@ Cypress.on('command:enqueued', (attrs) => {
if (args.includes('test_observability_log') || args.includes('test_observability_command')) return;
const message = args.reduce((result, logItem) => {
if (typeof logItem === 'object') {
return [result, JSON.stringify(logItem)].join(' ');
return [result, JSON.stringify(logItem, getCircularReplacer())].join(' ');
Copy link
Copy Markdown
Collaborator

@xxshubhamxx xxshubhamxx Jun 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Warning — [GRACEFUL-DEGRADATION] command:enqueued stringify has no try/catch

Problem

The two eventsQueue flush sites (beforeEach / afterEach) go through sanitizeForTask(), which has an outer try/catch and fully honours the graceful-degradation contract. This call site at line 239 uses JSON.stringify(logItem, getCircularReplacer()) directly — no try/catch.

If JSON.stringify throws for any non-circular reason — a toJSON() method that throws, a BigInt value (which JSON.stringify refuses), or a Proxy object whose traps throw inside WeakSet.has() — the exception escapes into Cypress's event loop, crashes the support file, and aborts the run. That is exactly the outcome this PR set out to prevent.

Suggested Fix

Replace JSON.stringify(logItem, getCircularReplacer()) with sanitizeForTask(logItem) and stringify the already-decycled plain-object result:

// Before (line 239)
return [result, JSON.stringify(logItem, getCircularReplacer())].join(' ');

// After
return [result, JSON.stringify(sanitizeForTask(logItem))].join(' ');

sanitizeForTask already wraps the serialize-and-decycle step in a try/catch, so this change makes the command:enqueued path as resilient as the flush paths.

Alternatively, keep the existing call and wrap it in a standalone try/catch that falls back to String(logItem):

let serialized;
try {
  serialized = JSON.stringify(logItem, getCircularReplacer());
} catch (e) {
  serialized = String(logItem);
}
return [result, serialized].join(' ');

Confidence: 🟢 Grounded in SDK Anti-Pattern #5 ("Missing Error Boundaries in Instrumentation") and the PR's own stated graceful-degradation contract.

}
return [result, logItem ? logItem.toString() : ''].join(' ');
}, '');
Expand Down Expand Up @@ -309,7 +337,7 @@ beforeEach(() => {

if (eventsQueue.length > 0) {
eventsQueue.forEach(event => {
cy.task(event.task, event.data, event.options);
cy.task(event.task, sanitizeForTask(event.data), event.options);
});
}
eventsQueue = [];
Expand All @@ -324,7 +352,7 @@ afterEach(function() {

if (eventsQueue.length > 0) {
eventsQueue.forEach(event => {
cy.task(event.task, event.data, event.options);
cy.task(event.task, sanitizeForTask(event.data), event.options);
});
}

Expand Down
Loading