Skip to content

Commit

Permalink
Merge pull request #1122 from owid/revert-fetch-with-retries
Browse files Browse the repository at this point in the history
Revert "enhance: retry fetch requests" #1066
  • Loading branch information
danielgavrilov authored Feb 1, 2022
2 parents ff83149 + d35c3af commit 92e9036
Show file tree
Hide file tree
Showing 7 changed files with 32 additions and 58 deletions.
12 changes: 3 additions & 9 deletions clientUtils/Util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -204,23 +204,17 @@ describe(retryPromise, () => {

it("resolves when promise succeeds first-time", async () => {
const promiseGetter = resolveAfterNthRetry(0, "success")
expect(retryPromise(promiseGetter, { maxRetries: 0 })).resolves.toEqual(
"success"
)
expect(retryPromise(promiseGetter, 1)).resolves.toEqual("success")
})

it("resolves when promise succeeds before retry limit", async () => {
const promiseGetter = resolveAfterNthRetry(2, "success")
expect(retryPromise(promiseGetter, { maxRetries: 2 })).resolves.toEqual(
"success"
)
expect(retryPromise(promiseGetter, 3)).resolves.toEqual("success")
})

it("rejects when promise doesn't succeed within retry limit", async () => {
const promiseGetter = resolveAfterNthRetry(3, "success")
expect(
retryPromise(promiseGetter, { maxRetries: 2 })
).rejects.toBeUndefined()
expect(retryPromise(promiseGetter, 3)).rejects.toBeUndefined()
})
})

Expand Down
60 changes: 21 additions & 39 deletions clientUtils/Util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -522,20 +522,23 @@ export const trimObject = <Obj>(
return clone
}

// TODO use fetchText() in fetchJSON()
// decided not to do this while implementing our COVID-19 page in order to prevent breaking something.
export const fetchText = async (url: string): Promise<string> => {
const response = await fetchWithRetries(url)
if (!response.ok) {
throw new Error(`${response.status} ${response.statusText}`)
}
return await response.text()
}

export const fetchWithRetries = async (
...params: Parameters<typeof fetch>
): ReturnType<typeof fetch> => {
return await retryPromise(() => fetch(...params), {
maxRetries: 3,
withDelay: true,
return new Promise((resolve, reject) => {
const req = new XMLHttpRequest()
req.addEventListener("load", function () {
resolve(this.responseText)
})
req.addEventListener("readystatechange", () => {
if (req.readyState === 4) {
if (req.status !== 200) {
reject(new Error(`${req.status} ${req.statusText}`))
}
}
})
req.open("GET", url)
req.send()
})
}

Expand Down Expand Up @@ -736,41 +739,20 @@ export const addDays = (date: Date, days: number): Date => {
return newDate
}

export async function wait(ms: number): Promise<void> {
return new Promise((resolve) => setTimeout(resolve, ms))
}

interface RetryParams {
maxRetries: number
withDelay: boolean
}
const defaultRetryParams: RetryParams = {
maxRetries: 2,
withDelay: false,
}
export async function retryPromise<T>(
promiseGetter: () => Promise<T>,
params?: Partial<RetryParams>
maxRetries: number = 3
): Promise<T> {
let retried = 0
const { maxRetries, withDelay } = {
...defaultRetryParams,
...params,
}
while (true) {
let lastError
while (retried++ < maxRetries) {
try {
return await promiseGetter()
} catch (error) {
if (retried >= maxRetries) {
throw error
}
if (withDelay) {
// Wait 100ms on first retry, 200ms on 2nd, 400ms on 3rd...
await wait(100 * 2 ** retried)
}
retried++
lastError = error
}
}
throw lastError
}

export function parseIntOrUndefined(s: string | undefined): number | undefined {
Expand Down
4 changes: 2 additions & 2 deletions explorer/ExplorerProgram.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { fetchWithRetries, trimObject } from "../clientUtils/Util"
import { trimObject } from "../clientUtils/Util"
import { GitCommit, SubNavId } from "../clientUtils/owidTypes"
import {
DefaultNewExplorerSlug,
Expand Down Expand Up @@ -365,7 +365,7 @@ export class ExplorerProgram extends GridProgram {
*/
private static tableDataLoader = new PromiseCache(
async (url: string): Promise<CoreTableInputOption> => {
const response = await fetchWithRetries(url)
const response = await fetch(url)
if (!response.ok) throw new Error(response.statusText)
const tableInput: CoreTableInputOption = url.endsWith(".json")
? await response.json()
Expand Down
3 changes: 1 addition & 2 deletions grapher/core/Grapher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ import {
debounce,
isInIFrame,
differenceObj,
fetchWithRetries,
} from "../../clientUtils/Util"
import { QueryParams } from "../../clientUtils/urls/UrlUtils"
import {
Expand Down Expand Up @@ -721,7 +720,7 @@ export class Grapher
)
this._receiveOwidDataAndApplySelection(json)
} else {
const response = await fetchWithRetries(this.dataUrl)
const response = await fetch(this.dataUrl)
if (!response.ok) throw new Error(response.statusText)
const json = await response.json()
this._receiveOwidDataAndApplySelection(json)
Expand Down
4 changes: 2 additions & 2 deletions site/EmbedChart.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import { Grapher } from "../grapher/core/Grapher"
import { GrapherFigureView } from "./GrapherFigureView"
import { deserializeJSONFromHTML } from "../clientUtils/serializers"
import { Url } from "../clientUtils/urls/Url"
import { fetchWithRetries } from "../clientUtils/Util"
import { excludeUndefined } from "../clientUtils/Util"

@observer
export class EmbedChart extends React.Component<{ src: string }> {
Expand All @@ -29,7 +29,7 @@ export class EmbedChart extends React.Component<{ src: string }> {
private async loadConfig() {
const { configUrl } = this
if (configUrl === undefined) return
const resp = await fetchWithRetries(configUrl)
const resp = await fetch(configUrl)
if (this.configUrl !== configUrl) {
// Changed while we were fetching
return
Expand Down
3 changes: 1 addition & 2 deletions site/covid/LastUpdated.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import React, { useEffect, useState } from "react"
import dayjs, { Dayjs } from "dayjs"
import relativeTime from "dayjs/plugin/relativeTime"
import { fetchWithRetries } from "../../clientUtils/Util"
dayjs.extend(relativeTime)

export interface LastUpdatedTokenProps {
Expand All @@ -13,7 +12,7 @@ export const LastUpdated = ({ timestampUrl }: LastUpdatedTokenProps) => {
useEffect(() => {
const fetchTimeStamp = async () => {
if (!timestampUrl) return
const response = await fetchWithRetries(timestampUrl)
const response = await fetch(timestampUrl)
if (!response.ok) return
const timestampRaw = await response.text()
const timestamp = timestampRaw.trim()
Expand Down
4 changes: 2 additions & 2 deletions site/stripe/DonateForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import {
} from "../../settings/clientSettings"

import stripe from "./stripe"
import { fetchWithRetries, stringifyUnkownError } from "../../clientUtils/Util"
import { stringifyUnkownError } from "../../clientUtils/Util"

type Interval = "once" | "monthly"

Expand Down Expand Up @@ -131,7 +131,7 @@ export class DonateForm extends React.Component {
}

const captchaToken = await this.getCaptchaToken()
const response = await fetchWithRetries(DONATE_API_URL, {
const response = await fetch(DONATE_API_URL, {
method: "POST",
credentials: "same-origin",
headers: {
Expand Down

0 comments on commit 92e9036

Please sign in to comment.