Instabug / Instabug-React-Native

In-app feedback and bug reporting tool for React Native
https://instabug.com/platforms/react-native
MIT License
309 stars 100 forks source link

[Network Logging] Response Body Mapping Change w/ v11+ #880

Closed tomwanzek closed 1 year ago

tomwanzek commented 1 year ago

Steps to Reproduce the Problem

Background

We are in the process of upgrading our project from Instabug RN v10.13.0 to RN v11+.

For Crash Reporting we obviously routinely rely on Network Logging to see if a particular API call failed. For privacy reasons we obfuscate (among other things) response bodies in production, unless the API call failed and the response body is just an non-sensitive error message.

In Staging and Dev we allow unobfuscated response bodies, given that all data are artificial and we can dig deeper in crash/bug replication investigations.

With Instabug RN v10.13.0:

The responseBody of the network data passed through obfuscation handler for NetworkLogger.setNetworkDataObfuscationHandler(...) was a (parsed JSON) object.

When checking a resulting network log in Instabug Crashes Occurrence, where we permitted unobfuscated response bodies, we would see the object tree of the JSON response. I.e. we could expand/collapse nested attributes in your UI.

With v11+:

This behavior seems to have changed. Now the responseBody is always a string, i.e. in our case a fully escaped stringified JSON object. The TypeScript definitions (🙇 😄 ) added w/ 11.5.x make it clear that the NetworkData responseBody is of type string | null as of now.

When checking a resulting network log in Instabug Crashes Occurrence Web UI, where we permitted unobfuscated response bodies, we now only find the fully escaped stringified JSON object.

Expected Behavior

Checking v11+ network logs in the Insabug UI, where we permit unobfuscated response bodies, should allow us to see the response object with attribute expand/collapse.

Actual Behavior

We only see the fully escaped stringified JSON object.

So not, sure how this would best be addressed, given that this seems to be deeper between the native-level integration and how the Instabug Crash Occurrence Web UI handles the network log it ultimately receives. I.e., is there a responsibility shift, where the Web UI, should check if the response body is legitimate JSON and can parsed into a displayable object with expand/collapse as before.

Instabug integration code

The only somewhat relevant code would be our network obfuscation handler. However the details are not really all that relevant, as the issue with object vs string applies for plainly passed through network data without any obfuscation manipulation.

SDK Version

Version we migrate from 10.13.0. Versions we checked the above behaviour with 11.3.0, 11.5.1., 11.6.0.

The TypeScript definitions added w/ 11.5.x make it clear that the NetworkData responseBody is of type string | null as of now.

React Native, iOS and Android Versions

RN: 068.5 we assume the behaviour is not Platform, OS version specific, but tested with

iOS: 16 Android: 11 and 13

in particular on Instabug RN v11.3.0

Device Model

All.

[Optional] Project That Reproduces the Issue

TheBuggedYRN commented 1 year ago

Thanks @tomwanzek for reporting this (and the great details)! We will investigate it further and get back to you.

TheBuggedYRN commented 1 year ago

This issue addresses two problems:

  1. The type of responseBody: Since version 11.0.0 we have added (in #710) JSON response type to the list of types we support. This requires the stringifying of JSON to be able to support it along with other types (text, blobs, documents, etc.). Knowing this, the type of responseBody is string | null as it's specified.
  2. Response is not expandable in the dashboard: This happens due to the incorrect type of response. You will need to set the responseType to "json" in case you are using XMLHttpRequest (or similar properties in other libraries). By doing so, you will have a non-escaped string of the response that you can parse back to the original response using JSON.parse. And of course it will solve the dashboard problem as well.

Feel free to open the issue again if you think this doesn't solve it. And do not hesitate to reach out if you need further help!

tomwanzek commented 1 year ago

@TheBuggedYRN Thanks for the quick response 🙇

That said, I have a few follow-up questions, which make me wonder whether the ticket should really be closed.

1) The JSON Rest API responses we were wondering about, have a completely accurate content-type set in the response header, which identifies them as application/json. This makes me wonder, whether the response header content-type should be driving how you treat the response, rather than responseType?

2) We are using Axios for the API calls in question, we are setting the accept request header in the process. That said, if I follow the suggestion above and set the responseType for the request handled by Axios, you are correct that the behavior of your network logger/obfuscation handler support changes:

tomwanzek commented 1 year ago

2) We are using Axios for the API calls in question, we are setting the accept request header in the process. That said, if I follow the suggestion above and set the responseType for the request handled by Axios, you are correct that the behavior of your network logger/obfuscation handler support changes:

  • On the upside, the Instabug Web UI will show unobfuscated responses again in object form, however,
  • the change in behavior seems to indicate that the typescript definitions for the NetworkData interface are not longer correct. I.e. in the scenario where responseType= "json" as per 2), the responseBody is no longer string | null it appears to me, but rather the parsed JSON object. Therefore any obfuscation handler logic would have to treat the "server-provided" unobfuscated responseBody as an object and also pass any "outbound" obfuscated response through as an object, rather than string.

Correction: I suspect the latter finding was mea culpa 🤦 , jumping between several project branches today, some with and some without Instabug 11.x. I might have inadvertently console logged some network data on a <11 branch.

So in short, it seems, when setting responseType explicitly the responseBody is a string within the network obfuscator, but does indeed get shown as an object in the Insabug UI.

TheBuggedYRN commented 1 year ago

@tomwanzek There might be a bit of confusion here, but the responseBody is always if tyoe string | null but the string might be "escaped" depending on the original responseType.

Here's an example that will help making things clear:

// If the responseType is "json"
// The response iteslf is object so when we stringify it,
// it gives a string that can be converted back to object using JSON.parse
const response    =  {"foo": "bar"};
const responseStr = JSON.stringify(response); // "{"foo":"bar"}" (string)
const responseObj = JSON.parse(responseStr);  // {"foo": "bar"}  (object)

// If the responseType is "text"
// The response iteslf is string so when we stringify it,
// it gives an escaped string that needs more parsing to get back to object
const response    =  "{"foo":"bar"}";
const responseStr = JSON.stringify(response); // ""{\"foo\":\"bar\"}"" (escaped string)
const responseObj = JSON.parse(JSON.parse(responseStr));  // {"foo": "bar"} (object)

So to reach your goal of having both an object inside setNetworkDataObfuscationHandler and in the dashboard, you can do the following:

  1. Explicitly set responseType to "json": We follow XMLHttpRequest spec, that why we depend on responseType instead of content-type header
  2. Parse responseBody inside setNetworkDataObfuscationHandler to get the original response as object
    NetworkLogger.setNetworkDataObfuscationHandler((data) => {
    const response = JSON.parse(data);
    // obfuscate data here...
    return obfuscatedData;
    });