fix(v10/browser): Release the XHR virtualError once the request completed

packages/browser-utils/src/instrument/xhr.ts

@@ -42,7 +42,7 @@ export function instrumentXHR(): void {
       //       have a stack trace. If you are using HttpClient integration,
       //       this is the expected behavior, as we are using this virtual error to capture
       //       the location of your XHR call, and group your HttpClient events accordingly.
-      const virtualError = new Error();
+      let virtualError: Error | undefined = new Error();
 
       const startTimestamp = timestampInSeconds() * 1000;
 
@@ -91,12 +91,13 @@ export function instrumentXHR(): void {
           };
           triggerHandlers('xhr', handlerData);
 
-          // In the `addEventListener` branch below, this handler is the only
-          // `readystatechange` listener we add, so detach it once the request is
-          // done to avoid pinning the XMLHttpRequest and its captured
-          // `virtualError` per HTTP call on long-lived pages. In the
-          // `onreadystatechange` proxy branch the handler isn't registered via
-          // `addEventListener`, so this is a harmless no-op there.
+          // An unformatted stack keeps its raw frames, and each frame keeps its receiver alive. For a request
+          // opened from the previous one's `readystatechange` callback that receiver is the previous
+          // XMLHttpRequest, so holding on would chain every completed request to the one still in flight.
+          virtualError = undefined;
+
+          // In the `addEventListener` branch below this is the only `readystatechange` listener we add, so
+          // detach it once the request is done. It's a no-op in the `onreadystatechange` proxy branch.
           xhrOpenThisArg.removeEventListener('readystatechange', onreadystatechangeHandler);
         }
       };

packages/browser-utils/test/instrument/xhrRetention.test.ts

@@ -0,0 +1,94 @@
+import { setFlagsFromString } from 'node:v8';
+import { runInNewContext } from 'node:vm';
+import { afterEach, describe, expect, it } from 'vitest';
+import { instrumentXHR } from '../../src/instrument/xhr';
+import { WINDOW } from '../../src/types';
+
+// Own file on purpose: instrumentation handlers live in a module-level registry that is never torn
+// down, so a handler from another test would retain the requests this one needs to see collected.
+
+const win = WINDOW as typeof WINDOW & { XMLHttpRequest?: typeof XMLHttpRequest };
+const originalXMLHttpRequest = win.XMLHttpRequest;
+
+function collectGarbage(): void {
+  setFlagsFromString('--expose-gc');
+  const gc = runInNewContext('gc') as () => void;
+  setFlagsFromString('--no-expose-gc');
+  gc();
+  gc();
+}
+
+class MockXMLHttpRequest {
+  public readyState: number = 0;
+  public status: number = 200;
+  private _listeners: Array<() => void> = [];
+
+  public addEventListener(_type: string, listener: () => void): void {
+    this._listeners.push(listener);
+  }
+
+  public removeEventListener(_type: string, listener: () => void): void {
+    this._listeners = this._listeners.filter(registered => registered !== listener);
+  }
+
+  public dispatch(): void {
+    // the SDK detaches its own listener while it runs, so iterate over a copy
+    for (const listener of this._listeners.slice()) {
+      // the browser calls listeners with the request as `this`, which is what puts the request into
+      // the stack frames of anything the listener calls
+      listener.call(this);
+    }
+  }
+
+  public open(_method: string, _url: string): void {}
+  public send(): void {}
+  public setRequestHeader(_header: string, _value: string): void {}
+}
+
+describe('instrumentXHR memory retention', () => {
+  afterEach(() => {
+    win.XMLHttpRequest = originalXMLHttpRequest;
+  });
+
+  it('does not retain completed requests that were chained from readystatechange', async () => {
+    win.XMLHttpRequest = MockXMLHttpRequest as unknown as typeof XMLHttpRequest;
+    instrumentXHR();
+
+    let firstRequest: WeakRef<MockXMLHttpRequest> | undefined;
+    // the request that never finishes stands in for the in-flight one, which the browser roots as a
+    // pending activity
+    let inFlightRequest: MockXMLHttpRequest | undefined;
+
+    const openRequest = (remaining: number): void => {
+      const xhr = new MockXMLHttpRequest();
+      xhr.open('GET', 'http://example.com');
+
+      if (!firstRequest) {
+        firstRequest = new WeakRef(xhr);
+      }
+
+      if (remaining === 0) {
+        inFlightRequest = xhr;
+        return;
+      }
+
+      xhr.addEventListener('readystatechange', function (this: MockXMLHttpRequest) {
+        if (this.readyState === 4) {
+          openRequest(remaining - 1);
+        }
+      });
+
+      xhr.readyState = 4;
+      xhr.dispatch();
+    };
+
+    openRequest(4);
+
+    // a WeakRef created in the current job is never cleared during that job
+    await new Promise(resolve => setTimeout(resolve, 0));
+    collectGarbage();
+
+    expect(inFlightRequest).toBeDefined();
+    expect(firstRequest?.deref()).toBeUndefined();
+  });
+});