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

Allow to disable interception of XMLHttpRequest #981

Closed
ddeath opened this issue May 22, 2023 · 4 comments
Closed

Allow to disable interception of XMLHttpRequest #981

ddeath opened this issue May 22, 2023 · 4 comments
Assignees

Comments

@ddeath
Copy link

ddeath commented May 22, 2023

I would like to have an option to completely prevent XMLHttpRequest to be intercepted. Current implementation of library will always enable it and then we have option to disable it with NetworkLogger.setEnabled(false)

However this is not ideal because if there are some other modifications to the same object, kind of undefined behaviour can happen. See example bellow.

In order to keep interception enabled by default I would suggest that you could add new option to InstabugConfig which would be true by default and if it would be false, XMLHttpRequest would be kept untouched.

In practice, this line: https://github.com/Instabug/Instabug-React-Native/blob/master/src/modules/Instabug.ts#L90 would be wrapped in IF based on that option.

Steps to Reproduce the Problem

  • Initialize instabug
  • Proxy XMLHttpRequest with custom code
  • Disable interception with NetworkLogger.setEnabled(false)

After these steps your own customization from step #2 is lost.

Expected Behavior

XMLHttpRequest is not proxied at all

Actual Behavior

XMLHttpRequest is first proxied and then original version is restored if it is disabled

SDK Version

11.9.1

React Native, iOS and Android Versions

0.71.8

Device Model

IOS Simulator

@a7medev a7medev self-assigned this May 24, 2023
@a7medev
Copy link
Contributor

a7medev commented May 24, 2023

Hi, @ddeath! Thanks for reporting this issue and providing this detailed description.

This is a pretty good point and I can see why this behavior occurs. The current implementation of the network interceptor grabs the original XMLHttpRequest methods that we alter from the beginning (once you import instabug-reactnative) so that when the user wants to disable network interception/logging using NetworkLogger.setEnabled(false) it reassigns these methods back to their original version.
The fact that we grab original XHR methods on import means that any changes done by the user after this import will get ignored even after disabling network logging.

I think a proper fix for this would be to get the original XMLHttpRequest methods when enabling it and not globally on import. This will enable users to patch XHR before calling Instabug.init or NetworkLogger.setEnabled(true).

I will proceed with the fix described and get back to you once ready.

@ddeath
Copy link
Author

ddeath commented May 24, 2023

Yeah, your solution will definitely help, however, I would also include that option to init object. Including that option to init object will cause more visibility for NetworkLogger and help identify these kinds of collisions between libraries sooner.

Thank you for working on this bug!

@a7medev
Copy link
Contributor

a7medev commented May 25, 2023

@ddeath I think introducing the change to get the XMLHttpRequest methods when needed then calling NetworkLogger.setEnabled(false) would have the same behavior as adding it as an option to init.
So to keep the APIs consistent, let's keep the logic of enabling/disabling network logging to NetworkLogger.

@a7medev
Copy link
Contributor

a7medev commented May 31, 2023

Since #984 has been merged with the fix, we can close this one.

@a7medev a7medev closed this as completed May 31, 2023
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

No branches or pull requests

2 participants