Fix response body capture for fetch telemetry

[waltjones]

Description of the change

Background

When enabled via the Rollbar config, network telemetry can capture the response body for fetch requests. When this was originally enabled, it worked in browser, but was finicky depending on the version of headless chrome used in the tests. Recently, the previously working test started failing.

Root cause

When the response body is a readable stream, the Rollbar handler must clone the response before reading the stream. The stream is locked once read, and cloning the stream allows safely reading it without reading the original. The code path already did this, but there's another requirement for response.clone(). It must be the first access to the response, or it will throw a TypeError.

The test started failing because headless chrome no longer accepted the readable stream used in the mock, and fails by throwing TypeError, mimicking the error described above. (Normally the browser would provide the readable stream, and will accept its own stream object.)

The fix

This PR changes the test mock to use a string for the response body, rather than a ReadableStream as was being used before. Previously, ReadableStream was the only way to get headless chrome to enforce the locking of the response body that we want to test. Currently, this behavior can be tested using a string in the mock, and both the first access requirement and single reader requirement are enforced and can be tested.

Type of change

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

Development

• Lint rules pass locally
• The code changed/added as part of this pull request has been covered with tests
• All tests related to the changed code pass in development

Code review

• This pull request has a descriptive title and information useful to a reviewer. There may be a screenshot or screencast attached
• "Ready for review" label attached to the PR and reviewers assigned
• Issue from task tracker has a link to this pull request
• Changes have been reviewed by at least one other engineer

[mudetroit]

Your logic all makes sense to me here, and great job with the explanation. I made one suggestion that may avoid what is probably a pretty rare edge case but perhaps something to consider.

[mudetroit]

Comment

Minor suggestion here. I don't think there are strong guarantees on the execution order of handlers to a single promise. Meaning in this case I don't think you can guarantee handlerA runs before handlerB:

function handlerA(response) { return response }
function handlerB(response) { return response }

var promise = new Promise(){...}

promise.then(handlerA)
promise.then(handlerB)

If your first handler returns the same response as the original (which yours does) you can simply return the promise returned by then to guarantee order

function handlerA(response) { return response }
function handlerB(response) { return response }

var promise = new Promise(){...}

var newPromise = promise.then(handlerA)
newPromise.then(handlerB)

[waltjones]

I agree this works based on existing browser behavior, not guaranteed. Great suggestion, I'll update the PR.

[mudetroit]

LGTM