Skip to content

Commit

Permalink
fix: no keep alive for large payloads (#1416)
Browse files Browse the repository at this point in the history
  • Loading branch information
pauldambra authored Sep 16, 2024
1 parent 1e18eea commit 2748a0d
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 6 deletions.
81 changes: 81 additions & 0 deletions src/__tests__/request.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ jest.mock('../utils/globals', () => ({
}))

import { fetch, XMLHttpRequest, navigator } from '../utils/globals'
import { uuidv7 } from '../uuidv7'

jest.mock('../config', () => ({ DEBUG: false, LIB_VERSION: '1.23.45' }))

Expand All @@ -23,6 +24,16 @@ const flushPromises = async () => {
jest.useRealTimers()
}

const bodyData = () => ({ key: uuidv7() })
const arrayOfBodyData = (n: number) => {
const arr = []
for (let i = 0; i < n; i++) {
arr.push(bodyData())
}
return arr
}
const veryLargeBodyData = arrayOfBodyData(8024)

describe('request', () => {
const mockedFetch: jest.MockedFunction<any> = fetch as jest.MockedFunction<any>
const mockedXMLHttpRequest: jest.MockedFunction<any> = XMLHttpRequest as jest.MockedFunction<any>
Expand Down Expand Up @@ -174,6 +185,76 @@ describe('request', () => {
text: 'oh no!',
})
})

describe('keepalive with fetch and large bodies can cause some browsers to reject network calls', () => {
it.each([
['always keepalive with small json POST', 'POST', 'small', undefined, true, ''],
[
'always keepalive with small gzip POST',
'POST',
'small',
Compression.GZipJS,
true,
'&compression=gzip-js',
],
[
'always keepalive with small base64 POST',
'POST',
'small',
Compression.Base64,
true,
'&compression=base64',
],
['never keepalive with GET', 'GET', undefined, Compression.GZipJS, false, '&compression=gzip-js'],
['never keepalive with large JSON POST', 'POST', veryLargeBodyData, undefined, false, ''],
[
'never keepalive with large GZIP POST',
'POST',
veryLargeBodyData,
Compression.GZipJS,
false,
'&compression=gzip-js',
],
[
'never keepalive with large base64 POST',
'POST',
veryLargeBodyData,
Compression.Base64,
false,
'&compression=base64',
],
])(
`uses keep alive: %s`,
(
_name: string,
method: 'POST' | 'GET',
body: any,
compression: Compression | undefined,
expectedKeepAlive: boolean,
expectedURLParams: string
) => {
request(
createRequest({
headers: {
'x-header': 'value',
},
method,
compression,
data: body,
})
)

expect(mockedFetch).toHaveBeenCalledWith(
`https://any.posthog-instance.com?ver=1.23.45&_=1700000000000${expectedURLParams}`,
expect.objectContaining({
headers: new Headers(),
keepalive: expectedKeepAlive,
method,
})
)
}
)
})
})

describe('adding query params to posthog API calls', () => {
Expand Down
26 changes: 20 additions & 6 deletions src/request.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { Compression, RequestOptions, RequestResponse } from './types'
import { formDataToQuery } from './utils/request-utils'

import { logger } from './utils/logger'
import { fetch, XMLHttpRequest, AbortController, navigator } from './utils/globals'
import { AbortController, fetch, navigator, XMLHttpRequest } from './utils/globals'
import { gzipSync, strToU8 } from 'fflate'

// eslint-disable-next-line compat/compat
Expand All @@ -17,6 +17,7 @@ const CONTENT_TYPE_FORM = 'application/x-www-form-urlencoded'
type EncodedBody = {
contentType: string
body: string | BlobPart
estimatedSize: number
}

export const extendURLParams = (url: string, params: Record<string, any>): string => {
Expand Down Expand Up @@ -45,24 +46,30 @@ const encodePostData = ({ data, compression }: RequestOptions): EncodedBody | un

if (compression === Compression.GZipJS) {
const gzipData = gzipSync(strToU8(JSON.stringify(data)), { mtime: 0 })
const blob = new Blob([gzipData], { type: CONTENT_TYPE_PLAIN })
return {
contentType: CONTENT_TYPE_PLAIN,
body: new Blob([gzipData], { type: CONTENT_TYPE_PLAIN }),
body: blob,
estimatedSize: blob.size,
}
}

if (compression === Compression.Base64) {
const b64data = _base64Encode(JSON.stringify(data))
const encodedBody = encodeToDataString(b64data)

return {
contentType: CONTENT_TYPE_FORM,
body: encodeToDataString(b64data),
body: encodedBody,
estimatedSize: new Blob([encodedBody]).size,
}
}

const jsonBody = JSON.stringify(data)
return {
contentType: CONTENT_TYPE_JSON,
body: JSON.stringify(data),
body: jsonBody,
estimatedSize: new Blob([jsonBody]).size,
}
}

Expand Down Expand Up @@ -107,7 +114,7 @@ const xhr = (options: RequestOptions) => {
}

const _fetch = (options: RequestOptions) => {
const { contentType, body } = encodePostData(options) ?? {}
const { contentType, body, estimatedSize } = encodePostData(options) ?? {}

// eslint-disable-next-line compat/compat
const headers = new Headers()
Expand All @@ -133,7 +140,14 @@ const _fetch = (options: RequestOptions) => {
fetch!(url, {
method: options?.method || 'GET',
headers,
keepalive: options.method === 'POST',
// if body is greater than 64kb, then fetch with keepalive will error
// see 8:10:5 at https://fetch.spec.whatwg.org/#http-network-or-cache-fetch,
// but we do want to set keepalive sometimes as it can help with success
// when e.g. a page is being closed
// so let's get the best of both worlds and only set keepalive for POST requests
// where the body is less than 64kb
// NB this is fetch keepalive and not http keepalive
keepalive: options.method === 'POST' && (estimatedSize || 0) < 64 * 1024,
body,
signal: aborter?.signal,
})
Expand Down

0 comments on commit 2748a0d

Please sign in to comment.