Skip to content

Commit

Permalink
fix: network capture tests should fail if we exhaust the body (#1395)
Browse files Browse the repository at this point in the history
* total spike

* A little tidier

* A little tidier

* test refactor

* simpler body reading

* wrap fetch before and after posthog to be sure

* remove fly-by change

* reimplement bug here

* but don't actually reimplement the bug
  • Loading branch information
pauldambra authored Aug 30, 2024
1 parent e04ba42 commit 0f9a413
Show file tree
Hide file tree
Showing 2 changed files with 145 additions and 49 deletions.
174 changes: 132 additions & 42 deletions cypress/e2e/session-recording.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,32 @@ function ensureActivitySendsSnapshots(initial = true) {
})
}

function wrapFetchInCypress({
originalFetch,
badlyBehaved = false,
}: {
originalFetch: (input: RequestInfo | URL, init?: RequestInit) => Promise<Response>
badlyBehaved?: boolean
}) {
return async function (requestOrURL: URL | RequestInfo, init?: RequestInit | undefined) {
// eslint-disable-next-line compat/compat
const req = new Request(requestOrURL, init)

const hasBody = typeof requestOrURL !== 'string' && 'body' in requestOrURL
if (hasBody) {
// we read the body to (maybe) exhaust it
badlyBehaved ? await requestOrURL.text() : await requestOrURL.clone().text()
}

const res = badlyBehaved ? await originalFetch(requestOrURL, init) : await originalFetch(req)

// we read the body to (maybe) exhaust it
badlyBehaved ? await res.text() : await res.clone().text()

return res
}
}

describe('Session recording', () => {
describe('array.full.js', () => {
it('captures session events', () => {
Expand Down Expand Up @@ -81,61 +107,125 @@ describe('Session recording', () => {
})
})
})
;[true, false].forEach((isBadlyBehavedWrapper) => {
describe(`network capture - when fetch wrapper ${
isBadlyBehavedWrapper ? 'is' : 'is not'
} badly behaved`, () => {
let originalFetch: typeof fetch | null = null

beforeEach(() => {
// wrap fetch to log the body of the request
// this simulates various libraries that require
// being able to read the request
// and possibly alter it
// see: https://github.com/PostHog/posthog/issues/24471
// for the catastrophic but hard to detect impact of
// interfering with that with our wrapper
// we wrap before PostHog and...
cy.window().then((win) => {
originalFetch = win.fetch
win.fetch = wrapFetchInCypress({ originalFetch, badlyBehaved: isBadlyBehavedWrapper })
})

describe('with network capture', () => {
beforeEach(() => {
start({
decideResponseOverrides: {
isAuthenticated: false,
sessionRecording: {
endpoint: '/ses/',
networkPayloadCapture: { recordBody: true },
start({
decideResponseOverrides: {
isAuthenticated: false,
sessionRecording: {
endpoint: '/ses/',
networkPayloadCapture: { recordBody: true },
},
capturePerformance: true,
autocapture_opt_out: true,
},
capturePerformance: true,
autocapture_opt_out: true,
},
url: './playground/cypress',
options: {
loaded: (ph) => {
ph.sessionRecording._forceAllowLocalhostNetworkCapture = true
url: './playground/cypress',
options: {
loaded: (ph) => {
ph.sessionRecording._forceAllowLocalhostNetworkCapture = true
},
},
},
})

cy.wait('@recorder')

cy.intercept({ url: 'https://example.com', times: 1 }, (req) => {
req.reply({
statusCode: 200,
headers: { 'Content-Type': 'application/json' },
body: {
message: 'This is a JSON response',
},
})
}).as('example.com')

// we wrap after PostHog
cy.window().then((win) => {
originalFetch = win.fetch
win.fetch = wrapFetchInCypress({ originalFetch, badlyBehaved: isBadlyBehavedWrapper })
})
})

cy.wait('@recorder')
})
afterEach(() => {
if (originalFetch) {
cy.window().then((win) => {
win.fetch = originalFetch
originalFetch = null
})
}
})

it('it sends network payloads', () => {
cy.intercept('https://example.com', 'success').as('example.com')
cy.get('[data-cy-network-call-button]').click()
cy.wait('@example.com')
cy.wait('@session-recording')
cy.phCaptures({ full: true }).then((captures) => {
const snapshots = captures.filter((c) => c.event === '$snapshot')

const capturedRequests: Record<string, any>[] = []
for (const snapshot of snapshots) {
for (const snapshotData of snapshot.properties['$snapshot_data']) {
if (snapshotData.type === 6) {
for (const req of snapshotData.data.payload.requests) {
capturedRequests.push(req)
it('it sends network payloads', () => {
cy.get('[data-cy-network-call-button]').click()
cy.wait('@example.com')
cy.wait('@session-recording')
cy.phCaptures({ full: true }).then((captures) => {
const snapshots = captures.filter((c) => c.event === '$snapshot')

const capturedRequests: Record<string, any>[] = []
for (const snapshot of snapshots) {
for (const snapshotData of snapshot.properties['$snapshot_data']) {
if (snapshotData.type === 6) {
for (const req of snapshotData.data.payload.requests) {
capturedRequests.push(req)
}
}
}
}
}

// yay, includes type 6 network data
expect(capturedRequests).to.have.length.above(0)
const expectedCaptureds: [RegExp, string][] = [
[/http:\/\/localhost:\d+\/playground\/cypress\//, 'navigation'],
[/http:\/\/localhost:\d+\/static\/array.js/, 'script'],
[
/http:\/\/localhost:\d+\/decide\/\?v=3&ip=1&_=\d+&ver=1\.\d\d\d\.\d+&compression=base64/,
'xmlhttprequest',
],
[/http:\/\/localhost:\d+\/static\/recorder.js\?v=1\.\d\d\d\.\d+/, 'script'],
[/https:\/\/example.com/, 'fetch'],
]

// yay, includes expected type 6 network data
expect(capturedRequests.length).to.equal(expectedCaptureds.length)
expectedCaptureds.forEach(([url, initiatorType], index) => {
expect(capturedRequests[index].name).to.match(url)
expect(capturedRequests[index].initiatorType).to.equal(initiatorType)
})

// the HTML file that cypress is operating on (playground/cypress/index.html)
// when the button for this test is click makes a post to https://example.com
const capturedFetchRequest = capturedRequests.find((cr) => cr.name === 'https://example.com/')
// the HTML file that cypress is operating on (playground/cypress/index.html)
// when the button for this test is click makes a post to https://example.com
const capturedFetchRequest = capturedRequests.find((cr) => cr.name === 'https://example.com/')
expect(capturedFetchRequest).to.not.be.undefined

expect(capturedFetchRequest.fetchStart).to.be.greaterThan(0) // proxy for including network timing info
expect(capturedFetchRequest.fetchStart).to.be.greaterThan(0) // proxy for including network timing info

expect(capturedFetchRequest.initiatorType).to.eql('fetch')
expect(capturedFetchRequest.isInitial).to.be.undefined
expect(capturedFetchRequest.requestBody).to.eq('i am the fetch body')
expect(capturedFetchRequest.initiatorType).to.eql('fetch')
expect(capturedFetchRequest.isInitial).to.be.undefined
expect(capturedFetchRequest.requestBody).to.eq('i am the fetch body')

expect(capturedFetchRequest.responseBody).to.eq(
JSON.stringify({
message: 'This is a JSON response',
})
)
})
})
})
})
Expand Down
20 changes: 13 additions & 7 deletions src/entrypoints/recorder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,13 +420,18 @@ function _tryReadBody(r: Request | Response): Promise<string> {
// eslint-disable-next-line compat/compat
return new Promise((resolve, reject) => {
const timeout = setTimeout(() => resolve('[SessionReplay] Timeout while trying to read body'), 500)
r.clone()
.text()
.then(
(txt) => resolve(txt),
(reason) => reject(reason)
)
.finally(() => clearTimeout(timeout))
try {
r.clone()
.text()
.then(
(txt) => resolve(txt),
(reason) => reject(reason)
)
.finally(() => clearTimeout(timeout))
} catch {
clearTimeout(timeout)
resolve('[SessionReplay] Failed to read body')
}
})
}

Expand Down Expand Up @@ -476,6 +481,7 @@ function initFetchObserver(
}
const recordRequestHeaders = shouldRecordHeaders('request', options.recordHeaders)
const recordResponseHeaders = shouldRecordHeaders('response', options.recordHeaders)

// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
const restorePatch = patch(win, 'fetch', (originalFetch: typeof fetch) => {
Expand Down

0 comments on commit 0f9a413

Please sign in to comment.