Skip to content
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.

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

Bug fix: interceptAnyObject used in monitorAPI #259

Merged
merged 2 commits into from
Oct 1, 2024

Conversation

ShirShintel
Copy link
Contributor

Bug Description

shell.getAPI returns empty objects for several APIs even though real implementation was contributed.

Problem

When the repluggable host is created with the disableMonitoring option off, monitorAPI is used to intercept and monitor any function in the API. This is done by contributing an intercepted API object instead of the contributed API. The intercepted API object is created using interceptAnyObject, that recursively intercepts all functions in the nested object and wraps them with monitoring logic.

The problem is that interceptAnyObject assumes the API is a plain object, as can be seen by its type. When passed an API that is not a plain object (e.g. a function), _.mapValues returns an empty object and the API implementation is lost.

The assumption that API is a plain object is wrong: APIs are not obligated to be built using a plain object, and can be really anything, from objects to functions to primitives. This can be seen in the TAPI type that makes no assumptions of the shape of the API.

Solution

  1. Changed the type of the inner argument in interceptAnyObject to be any, and instead added a type guard to make sure we only intercept plain objects.
  2. Replaced the _.isObjectLike check with _.isPlainObject which is more restrictive (will return false for arrays, maps, etc)
  3. Added sanity tests

@ShirShintel ShirShintel merged commit 8b95cf2 into master Oct 1, 2024
1 check passed
@ShirShintel ShirShintel deleted the fix-intercept-bug branch October 1, 2024 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants