Fixes encoding of circular referenced object / complex object

[DavidWeiss2]

Description

This PR enhances Firebase functions v2 by fixing an issue related to encoding objects with self-references, which previously led to "Maximum call stack size exceeded" errors. The change is to throw a clear error on what is the error (not JSON serializable, with object ref in console).

For a practical demonstration, consider a class instance with self-referencing properties. The updated encoding function now throws Data cannot be encoded in JSON: [object: object] in such cases, preventing stack overflow errors and ensuring the encoded output reflects the object's structure accurately, without infinite recursion. This improvement is verified through comprehensive test case included in the PR.

It fixes #1527

Code sample

See test cases.
The code before the change will give "Maximum call stack size exceeded" even on the basic case:

    class TestClass {
      foo: string;
      bar: number;
      self: TestClass;
      constructor(foo: string, bar: number) {
        this.foo = foo;
        this.bar = bar;
        this.self = this;
      }
    }
    const testObject = new TestClass("hello", 1);
    expect(()=>https.encode(testObject)).to.throw.(`Data cannot be encoded in JSON: ${testObject}`);

[DavidWeiss2]

Refactored to reflect team decision on throwing in the case of self ref.
#1527 (comment)
@inlined

[DavidWeiss2]

@inlined Can we get update on this PR status?

[DavidWeiss2]

Bump

[Copilot]

Pull Request Overview

This PR adds circular reference detection to the encode function to prevent infinite recursion when encoding objects with self-references.

• Changes the encode function signature to accept unknown instead of any for better type safety
• Implements circular reference detection using a WeakSet to track visited objects during encoding
• Adds a test case to verify that objects with self-references throw an appropriate error

Reviewed Changes

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

File	Description
src/common/providers/https.ts	Adds circular reference detection logic to the encode function using a WeakSet, refactors the object encoding logic, and adds a helper function isFunctionOrObject
spec/common/providers/https.spec.ts	Adds a test case verifying that objects with self-references throw an error

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The type definition object|((...args:any[])=>any) is overly verbose and redundant. Functions are already objects in JavaScript, so WeakSet<object> would suffice. Additionally, the type annotation is unnecessary here since TypeScript can infer it.

Suggested change

	const SELF_REFERENCE_WEAKSET = new WeakSet<object|((...args:any[])=>any)>();
	const SELF_REFERENCE_WEAKSET = new WeakSet();

[Copilot]

Missing spaces around operators. Should be if (!isFunctionOrObject(data) || SELF_REFERENCE_WEAKSET.has(data)) for consistency with project style.

Suggested change

	if(!isFunctionOrObject(data)||SELF_REFERENCE_WEAKSET.has(data)) {
	if (!isFunctionOrObject(data) || SELF_REFERENCE_WEAKSET.has(data)) {

[Copilot]

The logic is inverted. The condition !isFunctionOrObject(data) returns true when data is NOT an object/function (which is correct to throw), but combining it with || SELF_REFERENCE_WEAKSET.has(data) means it will also throw when a circular reference is detected AND when data is not an object. This breaks the existing test at line 967 which expects functions to encode as empty objects {}. The condition should be split: handle non-object/function first, then check for circular references separately.

Suggested change

	if(!isFunctionOrObject(data)||SELF_REFERENCE_WEAKSET.has(data)) {
	logger.error("Data cannot be encoded in JSON.", data);
	throw new Error(`Data cannot be encoded in JSON: ${data}`);
	}
	if (!isFunctionOrObject(data)) {
	logger.error("Data cannot be encoded in JSON.", data);
	throw new Error(`Data cannot be encoded in JSON: ${data}`);
	}
	if (SELF_REFERENCE_WEAKSET.has(data)) {
	logger.error("Data cannot be encoded in JSON (circular reference detected).", data);
	throw new Error(`Data cannot be encoded in JSON (circular reference detected): ${data}`);
	}

[Copilot]

The type predicate object|((...args:any[])=>any) is redundant. Since functions are objects in JavaScript, the predicate should be data is object. Additionally, this function could be simplified to a single line: return typeof data === 'object' || typeof data === 'function';

Suggested change

	function isFunctionOrObject(data: unknown): data is object|((...args:any[])=>any) {
	const isObjectOrFunction = typeof data === "object" || typeof data === "function";
	if (!isObjectOrFunction) {
	// If we got this far, the data is not encodable.
	return false;
	}
	return true;
	function isFunctionOrObject(data: unknown): data is object {
	return typeof data === "object" || typeof data === "function";

[Copilot]

This function is unnecessarily complex. The comment 'If we got this far, the data is not encodable' is misleading since the function just checks if data is an object or function, not whether it's encodable. The entire function can be simplified to: return typeof data === 'object' || typeof data === 'function';

Suggested change

	const isObjectOrFunction = typeof data === "object" || typeof data === "function";
	if (!isObjectOrFunction) {
	// If we got this far, the data is not encodable.
	return false;
	}
	return true;
	return typeof data === "object" || typeof data === "function";

[Copilot]

Missing spaces around the arrow function. Should be () => https.encode(testObject) for consistency with project style (see line 968).

Suggested change

	expect(()=>https.encode(testObject)).to.throw(`Data cannot be encoded in JSON: ${testObject}`);
	expect(() => https.encode(testObject)).to.throw(`Data cannot be encoded in JSON: ${testObject}`);

[Copilot]

Using a module-level WeakSet for tracking circular references creates a potential issue in concurrent request scenarios. If multiple encode calls execute simultaneously (common in serverless environments), they will share the same WeakSet, potentially causing false positives where an object from one request is detected as circular in another. The WeakSet should be passed as a parameter through recursive calls or use a different approach for circular reference detection.