refactor: make improvements to NetworkObserver

[daniel-graham-amplitude]

PROBLEMS

1. "networkObserver" does computations to calculate request body size and response body size. Although the computations are fairly basic, this could introduce a unnecessary overhead when an application does a very large amount of fetches Performance issue

2. our current "fetch" override implementation is too strongly typed and doesn't represent how it could be used in a browser (e.g.: we call input.toString() but input may be null or undefined when it's called in the wild) bug

3. our fetch override doesn't parse URL and HTTP Method when the first param is of type Request, which has a ".url" and ".method" attribute. bug

4. restoring "fetch" to it's original fetch after unsubscribing is dangerous because if another library also overrided fetch after us, then it's implementation will get wiped out too. bug

5. the request "Headers" type was being cast as a Record<string, string> but it can also be a Headers instance or an array of arrays (e.g.: [[<header>, <value>],...]) bug

6. there's some redundancy in the "try" and "catch" blocks where 'originalFetch' is called (same code being called)

SOLUTIONS

1. refactor "networkObserver" so that it doesn't calculate requestBodySize or responseBodySize, but rather the downstream subscribers can calculate those numbers, if they need them.

2. refactor the "fetch" override so that it accepts the 2 parameters as "optional" and then safely accesses everything (e.g.: replace input.toString() with input?.toString?.()

3. if the first parameter is of type Request, get URL from "input.url" and get method from "input.method". If not, fallback to url = "input?.toString?.()" and method = init.method (init is 2nd parameter) If "input.method" and "init.method" are both set, default to "init.method" (as per the spec)

4. don't restore fetch even when all callbacks are unsubscribed. Leave it in an overriden state so that we don't mess with others.

5. handle Arrays and Headers in the get headers() method of the request wrapper

6. move the NetworkRequestEvent creation after the original fetch is called, and then return/throw the original response/error that the original fetch returned/throws

COMMENTS

• This implementation of the fetch override works on the assumption that everything could be undefined or throw an exception and we need to handle it.

e.g.) the fetch override looks like this (simplified)

window.fetch = function (input, init) {
  try {
    beforeScript();
  } catch (e) {
    // suppress any error that beforeScript could possibly throw
  }
  
 let res, error;
  try {
    res = await originalFetch();
  } catch (e) {
    error = e;
  }
  
  try { 
    afterScript();
  } catch {}
    // suppress any error that afterScript could possibly throw
  }
  
  if (res) return res;
  else throw error;

Checklists

• Does your PR title have the correct title format?
• Does your PR have a breaking change?:

[promptless]

✅ No documentation updates required.

[promptless]

✅ No documentation updates required.

[Copilot]

Pull Request Overview

This PR refactors network event handling so that the networkObserver no longer performs computations for request/response body sizes and the fetch override safely handles optional parameters. Key changes include:

• Eliminating in-observer computations by deferring header and body size calculations to downstream consumers.
• Updating the fetch override to safely access parameters using optional chaining.
• Adjusting tests and utilities to support the new event payload structure.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
packages/session-replay-browser/test/session-replay.test.ts	Updated tests to verify header conversion and safe fetch override behavior.
packages/session-replay-browser/src/session-replay.ts	Added headerToObject helper and removed direct computations for body sizes.
packages/plugin-network-capture-browser/test/autocapture-plugin/track-network-event.test.ts	Modified tests for new event processing and safe parameter handling.
packages/plugin-network-capture-browser/src/track-network-event.ts	Updated logic to calculate body sizes using dedicated utilities.
packages/analytics-core/test/network-observer.test.ts	Refactored tests to validate changes in fetch override and event payload.
packages/analytics-core/src/network-observer.ts	Adjusted fetch override to safely handle optional input and removed in-observer computations.

[Copilot]

Pull Request Overview

This PR refactors the network observer implementation to remove internal computations for request and response body sizes and instead delegate these computations downstream. Key changes include:

• Removing computations in networkObserver and updating the event callback in SessionReplay.
• Adjusting the fetch override to safely handle optional parameters and Headers conversion.
• Updating tests across the session-replay, network-capture, and analytics-core packages to reflect these changes.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/session-replay-browser/test/session-replay.test.ts	Updated tests to verify Headers conversion and removal of body size computations.
packages/session-replay-browser/src/session-replay.ts	Added headerToObject helper and modified event callback to remove requestBody and convert headers.
packages/plugin-network-capture-browser/test/autocapture-plugin/track-network-event.test.ts	Revised tests to expect updated header objects and adjusted requestBodySize expectations.
packages/plugin-network-capture-browser/src/track-network-event.ts	Introduced getRequestBodyLength and calculateResponseBodySize improvements with safe handling.
packages/analytics-core/test/network-observer.test.ts	Modified tests for the new fetch override behavior and header handling.
packages/analytics-core/src/network-observer.ts	Refactored the fetch override to handle optional input and remove internal header conversion calculations.

[Copilot]

Pull Request Overview

This PR refactors the network observer and fetch override to remove unnecessary computations and shift body size calculations into dedicated wrapper classes while updating test cases accordingly.

• Use RequestWrapper and ResponseWrapper to compute header and body size information.
• Update tests to reflect the changes in event serialization and fetch override behavior.
• Refactor the fetch override to avoid restoring the original fetch when unsubscribing.

Reviewed Changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.

File	Description
packages/plugin-network-capture-browser/test/autocapture-plugin/track-network-event.test.ts	Adjusted tests to use new wrapper objects and updated expectations for network event properties.
packages/plugin-network-capture-browser/src/track-network-event.ts	Updated event creation to use requestWrapper/responseWrapper and switched to using parseUrl with optional input.
packages/analytics-core/src/network-observer.ts	Refactored the fetch override, introduced RequestWrapper/ResponseWrapper classes, and removed fetch restoration logic.

Comments suppressed due to low confidence (2)

packages/plugin-network-capture-browser/test/autocapture-plugin/track-network-event.test.ts:39

• [nitpick] The use of 'as any' in test cases hides potential type mismatches. Replacing these casts with explicit types could improve test clarity and long-term maintainability.

public requestWrapper = { bodySize: 100, headers: { 'Content-Type': 'application/json' } } as any,

packages/plugin-network-capture-browser/src/track-network-event.ts:160

• Since the type casting to NetworkRequestEvent has been removed in favor of using wrapper objects, ensure that downstream consumers handle cases where requestWrapper or responseWrapper might be undefined, to avoid unexpected runtime errors.

const request = networkEvent.event;

[jxiwang]

This would essentially return undefined to the event property?

[daniel-graham-amplitude]

That's correct yes. Essentially it's "giving up" once it's reached the limit and not computing the size.

[jxiwang]

The performance concerns here are that the size calculation can bottleneck the request?

[daniel-graham-amplitude]

The performance concerns here are that the size calculation can bottleneck the request?

The size calculations and also the construction of the "requestHeaders".

autocapture.networkTracking filters out requests. This change restricts the calculation of the size to only the requests and responses that aren't being filtered. Which, in normal circumstances, would be >99% of requests (ie: requests with an HTTP status > 500)

[jxiwang]

I think this still works, but do we want any additional header checks as well to be safe?

[daniel-graham-amplitude]

According to specification, a Headers object is the only type that is set in the Response.headers object so we shouldn't expect there to ever be a Record or Array type (like what request has).

[jxiwang]

What about the off chance that is undefined?

[daniel-graham-amplitude]

Yeah good thinking. I'll add a defensive check to it.

[jxiwang]

This is the original error thrown by the wrapped response?

[daniel-graham-amplitude]

That is correct yes. This is where the original error gets caught

// 2. make the call to the original fetch and preserve the response or error
let originalResponse, originalError;
try {
  originalResponse = await originalFetch(requestInfo as RequestInfo | URL, requestInit);
} catch (err) {
  // Capture error information
  originalError = err;
}

It's essential that originalResponse is the only object that is ever returned and that originalError is the only erro that is ever thrown.

[Copilot]

Pull Request Overview

This PR refactors network observation and event tracking to address performance overhead and type safety issues in the fetch override.

• Removed redundant body size calculations by introducing request and response wrappers.
• Refactored the fetch override to safely handle optional inputs and to prevent unsafe restoration of the original fetch.
• Updated tests and exports to reflect the new modular structure for network events.

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
packages/plugin-network-capture-browser/test/autocapture-plugin/track-network-event.test.ts	Updated tests to verify the new wrapper structures and serialization logic.
packages/plugin-network-capture-browser/src/track-network-event.ts	Adjusted event tracking to use wrapper properties for body sizes.
packages/analytics-core/src/network-request-event.ts	Introduced RequestWrapper and ResponseWrapper classes for improved encapsulation.
packages/analytics-core/src/network-observer.ts	Restructured the fetch override and event handling to improve performance and type safety.
packages/analytics-core/src/index.ts	Updated exports to align with the refactored network event modules.

Comments suppressed due to low confidence (1)

packages/plugin-network-capture-browser/test/autocapture-plugin/track-network-event.test.ts:38

• [nitpick] Avoid using 'as any' in test assertions if possible, or add a comment explaining its necessity, to maintain type clarity in tests.

}) as any,

[Copilot]

If performance timestamps are unavailable and the event is silently skipped, consider logging a debug message to indicate why the network event callback was not triggered.

Suggested change

	// so we can't construct a NetworkRequestEvent
	// so we can't construct a NetworkRequestEvent
	this.logger?.debug('Network event callback skipped: performance timestamps are unavailable. This may indicate that the performance API is not supported in the current environment.');

[Copilot]

[nitpick] The use of optional chaining on built-in functions (e.g., Date.now?() and performance.now?()) in getTimestamps() is unconventional; since these functions are standard in browsers, removing optional chaining would make the code clearer.

[jxiwang]

Overall LGTM! Thanks for doing this! As long as we are minimizing the effect on the customer's network requests, then we should be good.

[daniel-graham-amplitude]

Overall LGTM! Thanks for doing this! As long as we are minimizing the effect on the customer's network requests, then we should be good.

Thanks! Yeah I'm mostly concerned about preserving fetch and making it future-proof so that nobody can break it (easily).