fix: #186 avoid infinite recursion in filter method

[miquelbeltran]

fix: #186 avoid infinite recursion in filter method

Description 📝

• Purpose: filterKeys method uses recursion to explore the provided object, this can potentially cause infinite recursion and eventually throw the "Maximum call stack size exceeded" error.
• Approach: Adds an explored parameter to the recursive filterKeys method, to check if the incoming object has already been explored or not.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Updates

• TDD approach: Created a test that can reproduce the issue, using a cyclic object, and causes a "Maximum call stack size exceeded" error, then implemented the solution to fix it.
• Implementation:
  • Added explored: Set<object> as parameter (it is null by default).
  • When iterating over the object children, if the child is in the explored Set, remove from parent. Same as when applying filters.
  • When calling to filterKeys recursively, include the explored object in the Set.
• After performing the changes, the test passes.
• Also added a test assert to ensure that the original object is not modified.

One decision I had to make, is if the self-referenced object should be included in the output or not.

My decision to remove it from the output, is that we cannot apply filtering to the properties of the self-referenced object. Remember that we are doing a copy because we don't want to modify the original.

example of input from the test:

{
  myself: *self reference*,
  key: "value",
  filtered: true,
  other: {
    body: *self reference*,
    filtered: true,
  }
}

output would be:

{
  key: "value",
  other: { }
}

After removing all filtered properties and self-referenced objects.

Test plan 🧪

• Method is unit tested with a new test, also existing tests pass.

Author to check 👓

• Project and all contained modules builds successfully
• Self-/dev-tested
• Unit/UI/Automation/Integration tests provided where applicable
• Code is written to standards
• Appropriate documentation written (code comments, internal docs)

Reviewer to check ✔️

• Project and all contained modules builds successfully
• Change has been dev-/reviewer-tested, where possible
• Unit/UI/Automation/Integration tests provided where applicable
• Code is written to standards
• Appropriate documentation written (code comments, internal docs)

[miquelbeltran]

explored is set to null by default, so it is not a mandatory parameter

[miquelbeltran]

Added to ensure that we do the copy in the filterKeys method, i.e. const _obj = { ...obj } as Record<string, object>;

[TheRealAgentK]

This looks good to me!

[sumitramanga]

Not sure if it's been checked but it may be useful to check around the repo to see if we've done infinite recursion in the past and fix that up?

[miquelbeltran]

I couldn't see any other instances in this repo. Do you mean in all the codebases from Raygun?

[sumitramanga]

LGTM! Just one comment to address this issue as a whole in the repo if that hasn't been done yet :)