Back off metrics uploads when the tracking endpoint is unreachable (#11683)
This commit is contained in:
committed by
GitHub
parent
52345ad3ca
commit
08069e2877
@@ -161,6 +161,75 @@ describe('MetricsClient', () => {
|
||||
expect(requestCount).toBe(2) // No additional requests
|
||||
})
|
||||
|
||||
it('backs off instead of retrying every flush when the endpoint is unreachable', async () => {
|
||||
let requestCount = 0
|
||||
|
||||
fetchMock.mockImplementation(() => {
|
||||
requestCount++
|
||||
return Promise.resolve({
|
||||
ok: false,
|
||||
status: 500,
|
||||
text: () => Promise.resolve('Internal Server Error'),
|
||||
})
|
||||
})
|
||||
|
||||
const client = new MetricsClient<TestEvents>()
|
||||
client.track('click', {button: 'first'})
|
||||
|
||||
await jest.advanceTimersByTimeAsync(10_000)
|
||||
expect(requestCount).toBe(1)
|
||||
|
||||
// No further requests go out during the backoff, even though the flush
|
||||
// interval keeps firing and events keep arriving.
|
||||
client.track('click', {button: 'during-backoff'})
|
||||
await jest.advanceTimersByTimeAsync(25_000)
|
||||
expect(requestCount).toBe(1)
|
||||
|
||||
// Backoff expires, and a single further attempt is made.
|
||||
await jest.advanceTimersByTimeAsync(15_000)
|
||||
expect(requestCount).toBe(2)
|
||||
|
||||
// That one fails too, so the backoff doubles.
|
||||
client.track('click', {button: 'after-second-failure'})
|
||||
await jest.advanceTimersByTimeAsync(40_000)
|
||||
expect(requestCount).toBe(2)
|
||||
})
|
||||
|
||||
it('caps the failed queue at maxBatchSize', async () => {
|
||||
fetchMock.mockImplementation(() =>
|
||||
Promise.resolve({
|
||||
ok: false,
|
||||
status: 500,
|
||||
text: () => Promise.resolve('Internal Server Error'),
|
||||
}),
|
||||
)
|
||||
|
||||
const client = new MetricsClient<TestEvents>()
|
||||
client.maxBatchSize = 5
|
||||
|
||||
// Exceeding maxBatchSize flushes all six events as one failing batch.
|
||||
for (let i = 0; i < 6; i++) {
|
||||
client.track('click', {button: `btn-${i}`})
|
||||
}
|
||||
await jest.advanceTimersByTimeAsync(0)
|
||||
|
||||
let retried: {payload: {button: string}}[] = []
|
||||
fetchMock.mockImplementation((_url: string, options: {body: string}) => {
|
||||
retried = (
|
||||
JSON.parse(options.body) as {events: {payload: {button: string}}[]}
|
||||
).events
|
||||
return Promise.resolve({ok: true, status: 200})
|
||||
})
|
||||
|
||||
appStateCallback('active')
|
||||
await jest.advanceTimersByTimeAsync(0)
|
||||
|
||||
// The oldest event was dropped rather than buffered indefinitely.
|
||||
expect(retried).toHaveLength(5)
|
||||
expect(retried[0].payload.button).toBe('btn-1')
|
||||
expect(retried[4].payload.button).toBe('btn-5')
|
||||
})
|
||||
|
||||
it('flushes when app goes to background', async () => {
|
||||
const client = new MetricsClient<TestEvents>()
|
||||
client.track('click', {button: 'submit'})
|
||||
|
||||
@@ -14,6 +14,15 @@ type Event<M extends Record<string, any>> = {
|
||||
const TRACKING_ENDPOINT = env.METRICS_API_HOST + '/t'
|
||||
const logger = Logger.create(Logger.Context.Metric, {})
|
||||
|
||||
/**
|
||||
* The tracking endpoint is unreachable for plenty of users - offline, or
|
||||
* blocked by a content blocker. Without a backoff every flush keeps firing,
|
||||
* and browsers coalesce the throttled background timers into a burst of
|
||||
* failing requests as soon as the tab is refocused.
|
||||
*/
|
||||
const MIN_BACKOFF_MS = 30_000
|
||||
const MAX_BACKOFF_MS = 5 * 60_000
|
||||
|
||||
export class MetricsClient<M extends Record<string, any>> {
|
||||
maxBatchSize = 100
|
||||
|
||||
@@ -21,6 +30,8 @@ export class MetricsClient<M extends Record<string, any>> {
|
||||
private queue: Event<M>[] = []
|
||||
private failedQueue: Event<M>[] = []
|
||||
private flushInterval: NodeJS.Timeout | null = null
|
||||
private backoffMs = 0
|
||||
private backoffUntil = 0
|
||||
|
||||
start() {
|
||||
if (this.started) return
|
||||
@@ -62,6 +73,12 @@ export class MetricsClient<M extends Record<string, any>> {
|
||||
|
||||
flush() {
|
||||
if (!this.queue.length) return
|
||||
if (Date.now() < this.backoffUntil) {
|
||||
// Endpoint is unreachable. Hold the most recent events so the queue
|
||||
// can't grow without bound while we wait for the backoff to expire.
|
||||
this.trim(this.queue)
|
||||
return
|
||||
}
|
||||
const events = this.queue.splice(0, this.queue.length)
|
||||
this.sendBatch(events)
|
||||
}
|
||||
@@ -94,10 +111,19 @@ export class MetricsClient<M extends Record<string, any>> {
|
||||
throw new Error(`${res.status} Failed to fetch — ${error}`)
|
||||
}
|
||||
}
|
||||
|
||||
this.backoffMs = 0
|
||||
this.backoffUntil = 0
|
||||
} catch (e: any) {
|
||||
if (isNetworkError(e)) {
|
||||
this.backoffMs = Math.min(
|
||||
this.backoffMs === 0 ? MIN_BACKOFF_MS : this.backoffMs * 2,
|
||||
MAX_BACKOFF_MS,
|
||||
)
|
||||
this.backoffUntil = Date.now() + this.backoffMs
|
||||
if (isRetry) return // retry once
|
||||
this.failedQueue.push(...events)
|
||||
this.trim(this.failedQueue)
|
||||
return
|
||||
}
|
||||
logger.error(`Failed to send metrics`, {
|
||||
@@ -111,4 +137,14 @@ export class MetricsClient<M extends Record<string, any>> {
|
||||
const events = this.failedQueue.splice(0, this.failedQueue.length)
|
||||
this.sendBatch(events, true)
|
||||
}
|
||||
|
||||
/**
|
||||
* Drop the oldest events so a queue can't grow without bound while the
|
||||
* endpoint is unreachable.
|
||||
*/
|
||||
private trim(queue: Event<M>[]) {
|
||||
if (queue.length > this.maxBatchSize) {
|
||||
queue.splice(0, queue.length - this.maxBatchSize)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user