fix: cancel the fetch

This commit is contained in:
Nolan Lawson 2024-03-17 10:47:53 -07:00
parent a627dbf933
commit 8b25eb668d
14 changed files with 80 additions and 64 deletions

View File

@ -6,13 +6,13 @@ import { isSignalAborted } from './utils/isSignalAborted.js'
export async function checkForUpdates (db, dataSource, signal) {
// just do a simple HEAD request first to see if the eTags match
let emojiData
let eTag = await getETag(dataSource)
let eTag = await getETag(dataSource, signal)
if (isSignalAborted(signal)) {
return
}
if (!eTag) { // work around lack of ETag/Access-Control-Expose-Headers
const eTagAndData = await getETagAndData(dataSource)
const eTagAndData = await getETagAndData(dataSource, signal)
if (isSignalAborted(signal)) {
return
}
@ -36,7 +36,7 @@ export async function checkForUpdates (db, dataSource, signal) {
} else {
console.log('Database update available')
if (!emojiData) {
const eTagAndData = await getETagAndData(dataSource)
const eTagAndData = await getETagAndData(dataSource, signal)
if (isSignalAborted(signal)) {
return
}
@ -48,7 +48,7 @@ export async function checkForUpdates (db, dataSource, signal) {
}
export async function loadDataForFirstTime (db, dataSource, signal) {
let [eTag, emojiData] = await getETagAndData(dataSource)
let [eTag, emojiData] = await getETagAndData(dataSource, signal)
if (isSignalAborted(signal)) {
return
}

View File

@ -7,9 +7,13 @@ function assertStatus (response, dataSource) {
}
}
export async function getETag (dataSource) {
export async function getETag (dataSource, signal) {
performance.mark('getETag')
const response = await fetch(dataSource, { method: 'HEAD' })
/* istanbul ignore if */
if (import.meta.env.MODE !== 'production' && !signal) {
throw new Error('signal must be defined')
}
const response = await fetch(dataSource, { method: 'HEAD', signal })
assertStatus(response, dataSource)
const eTag = response.headers.get('etag')
warnETag(eTag)
@ -17,9 +21,13 @@ export async function getETag (dataSource) {
return eTag
}
export async function getETagAndData (dataSource) {
export async function getETagAndData (dataSource, signal) {
performance.mark('getETagAndData')
const response = await fetch(dataSource)
/* istanbul ignore if */
if (import.meta.env.MODE !== 'production' && !signal) {
throw new Error('signal must be defined')
}
const response = await fetch(dataSource, { signal })
assertStatus(response, dataSource)
const eTag = response.headers.get('etag')
warnETag(eTag)

View File

@ -68,7 +68,8 @@ export default class PickerElement extends HTMLElement {
const { database } = this._ctx
database.close()
// only happens if the database failed to load in the first place, so we don't care
// only happens if the database failed to load in the first place, or if the
// fetch is aborted while inflight, so we don't care
.catch(err => console.error(err))
}
})

View File

@ -22,14 +22,14 @@ describe('database tests', () => {
await db.ready()
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.close()
db = new Database({ dataSource: ALL_EMOJI })
await db.ready()
await tick(5) // the HEAD request is done asynchronously, so wait for it
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toEqual({ method: 'HEAD' })
expect(fetch.lastOptions().method).toBe('HEAD')
await db.delete()
})
@ -38,16 +38,16 @@ describe('database tests', () => {
await db.ready()
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI_NO_ETAG)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.close()
db = new Database({ dataSource: ALL_EMOJI_NO_ETAG })
await db.ready()
await tick(5) // the request is done asynchronously, so wait for it
expect(fetch.calls().length).toBe(3)
expect(fetch.calls().at(-2)[0]).toBe(ALL_EMOJI_NO_ETAG)
expect(fetch.calls().at(-2)[1]).toEqual({ method: 'HEAD' })
expect(fetch.calls().at(-2)[1].method).toBe('HEAD')
expect(fetch.lastUrl()).toBe(ALL_EMOJI_NO_ETAG)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.delete()
})
@ -56,13 +56,13 @@ describe('database tests', () => {
await db.ready()
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.delete()
db = new Database({ dataSource: ALL_EMOJI })
await db.ready()
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.delete()
})
@ -71,16 +71,16 @@ describe('database tests', () => {
await db.ready()
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI_MISCONFIGURED_ETAG)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.close()
db = new Database({ dataSource: ALL_EMOJI_MISCONFIGURED_ETAG })
await db.ready()
await tick(5) // the request is done asynchronously, so wait for it
expect(fetch.calls().length).toBe(3)
expect(fetch.calls().at(-2)[0]).toBe(ALL_EMOJI_MISCONFIGURED_ETAG)
expect(fetch.calls().at(-2)[1]).toEqual({ method: 'HEAD' })
expect(fetch.calls().at(-2)[1].method).toBe('HEAD')
expect(fetch.lastUrl()).toBe(ALL_EMOJI_MISCONFIGURED_ETAG)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.delete()
})

View File

@ -30,6 +30,6 @@ describe('basic fetch tests', () => {
expect(resp.headers.get('etag')).toBe('W/xxx')
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toEqual({ method: 'HEAD' })
expect(fetch.lastOptions().method).toBe('HEAD')
})
})

View File

@ -52,23 +52,23 @@ describe('database second load and update', () => {
// first load
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(dataSource)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect((await db.getEmojiByShortcode('rofl')).annotation).toBe('rolling on the floor laughing')
expect(await db.getEmojiByShortcode('weary_cat')).toBeFalsy()
}, async (db, dataSource) => {
// second load
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(dataSource)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(fetch.calls().at(-2)[0]).toBe(dataSource)
expect(fetch.calls().at(-2)[1]).toEqual({ method: 'HEAD' })
expect(fetch.calls().at(-2)[1].method).toBe('HEAD')
expect((await db.getEmojiByShortcode('rofl'))).toBeFalsy()
expect((await db.getEmojiByShortcode('pineapple')).annotation).toBe('pineapple')
}, async (db, dataSource) => {
// third load
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(dataSource)
expect(fetch.lastOptions()).toEqual({ method: 'HEAD' })
expect(fetch.lastOptions().method).toBe('HEAD')
expect((await db.getEmojiByShortcode('rofl'))).toBeFalsy()
expect((await db.getEmojiByShortcode('pineapple')).annotation).toBe('pineapple')
})
@ -138,7 +138,7 @@ describe('database second load and update', () => {
await db.ready()
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(dataSource)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect((await db.getEmojiByShortcode('rofl')).annotation).toBe('rolling on the floor laughing')
expect(await db.getEmojiByShortcode('weary_cat')).toBeFalsy()
@ -157,9 +157,9 @@ describe('database second load and update', () => {
await db._lazyUpdate
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(dataSource)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(fetch.calls().at(-2)[0]).toBe(dataSource)
expect(fetch.calls().at(-2)[1]).toEqual({ method: 'HEAD' })
expect(fetch.calls().at(-2)[1].method).toBe('HEAD')
expect((await db.getEmojiByShortcode('rofl'))).toBeFalsy()
expect((await db.getEmojiByShortcode('pineapple')).annotation).toBe('pineapple')
await db.close()
@ -172,9 +172,9 @@ describe('database second load and update', () => {
await db._lazyUpdate
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(dataSource)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(fetch.calls().at(-2)[0]).toBe(dataSource)
expect(fetch.calls().at(-2)[1]).toEqual({ method: 'HEAD' })
expect(fetch.calls().at(-2)[1].method).toBe('HEAD')
expect((await db.getEmojiByShortcode('rofl'))).toBeFalsy()
expect((await db.getEmojiByShortcode('pineapple')).annotation).toBe('pineapple')
await db.delete()
@ -191,7 +191,7 @@ describe('database second load and update', () => {
await db.ready()
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(dataSource)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect((await db.getEmojiByShortcode('rofl')).annotation).toBe('rolling on the floor laughing')
expect(await db.getEmojiByShortcode('weary_cat')).toBeFalsy()
@ -211,9 +211,9 @@ describe('database second load and update', () => {
await db._lazyUpdate
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(dataSource2)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(fetch.calls().at(-2)[0]).toBe(dataSource2)
expect(fetch.calls().at(-2)[1]).toEqual({ method: 'HEAD' })
expect(fetch.calls().at(-2)[1].method).toBe('HEAD')
expect((await db.getEmojiByShortcode('rofl'))).toBeFalsy()
expect((await db.getEmojiByShortcode('pineapple')).annotation).toBe('pineapple')
@ -226,7 +226,7 @@ describe('database second load and update', () => {
await db._lazyUpdate
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(dataSource2)
expect(fetch.lastOptions()).toEqual({ method: 'HEAD' })
expect(fetch.lastOptions().method).toBe('HEAD')
expect((await db.getEmojiByShortcode('rofl'))).toBeFalsy()
expect((await db.getEmojiByShortcode('pineapple')).annotation).toBe('pineapple')
@ -248,7 +248,7 @@ describe('database second load and update', () => {
await tick(40) // the request is done asynchronously, so wait for it
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(otherSource)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await db.delete()
})

View File

@ -46,7 +46,15 @@ function runTest ({ secondLoad, dataChanged, dataSource, signalAbortedCallCount
const db2 = new Database({ dataSource })
await waitForSignalAbortCalledNTimes(signalAbortedCallCount)
await db2.close()
const doClose = async () => {
await db2.close()
}
if (!secondLoad && signalAbortedCallCount === 2) {
// this happens to cancel an inflight fetch request
await expect(doClose).rejects.toThrow(/The operation was aborted/)
} else {
await doClose()
}
await tick(40)
})
}
@ -79,12 +87,11 @@ describe('database timing tests', () => {
}
]
scenarios.forEach(({ testName, dataSource, maxExpectedSignalAbortedCallCount }) => {
// Number of times somebody called the getter on `signal.aborted` which
// we are using as an easy way to get full code coverage here
const signalAbortedCallCounts = new Array(maxExpectedSignalAbortedCallCount).fill().map((_, i) => i)
signalAbortedCallCounts.forEach(signalAbortedCallCount => {
describe(testName, () => {
describe(testName, () => {
// Number of times somebody called the getter on `signal.aborted` which
// we are using as an easy way to get full code coverage here
const signalAbortedCallCounts = new Array(maxExpectedSignalAbortedCallCount).fill().map((_, i) => i)
signalAbortedCallCounts.forEach(signalAbortedCallCount => {
runTest({ secondLoad, dataChanged, dataSource, signalAbortedCallCount })
})
})

View File

@ -30,19 +30,19 @@ describe('Picker tests', () => {
picker = new Picker({ dataSource: ALL_EMOJI, locale: 'en' })
document.body.appendChild(picker)
container = picker.shadowRoot.querySelector('.picker')
await tick(20)
await tick(40)
await waitFor(() => expect(
testingLibrary.getAllByRole(getByRole('tabpanel'), 'menuitem')).toHaveLength(numInGroup1),
{ timeout: 2000 }
)
await tick(20)
await tick(40)
})
afterEach(async () => {
await tick(20)
await tick(40)
document.body.removeChild(picker)
await tick(20)
await tick(40)
await new Database({ dataSource: ALL_EMOJI, locale: 'en' }).delete()
await tick(20)
await tick(40)
await basicAfterEach()
})
@ -225,7 +225,7 @@ describe('Picker tests', () => {
test('measures zwj emoji', async () => {
getByRole('tab', { name: 'Flags' }).click()
await tick(20)
await tick(40)
await waitFor(() => expect(testingLibrary.getAllByRole(getByRole('tabpanel'), 'menuitem'))
.toHaveLength(truncatedEmoji.filter(_ => _.group === 9).length))
})

View File

@ -33,7 +33,7 @@ describe('attributes tests', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(FR_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(picker.locale).toEqual('fr')
expect(picker.dataSource).toEqual(FR_EMOJI)
@ -172,7 +172,7 @@ describe('attributes tests', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
document.body.removeChild(div)
await tick(20)

View File

@ -18,7 +18,7 @@ describe('constructor', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
document.body.removeChild(picker)
await tick(20)

View File

@ -56,7 +56,7 @@ describe('element tests', () => {
await tick(120)
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await type(getByRole(container, 'combobox'), 'monkey face')
await waitFor(() => expect(getAllByRole(container, 'option')).toHaveLength(1), {
timeout: 2000
@ -68,7 +68,7 @@ describe('element tests', () => {
await tick(120)
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(FR_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await clear(getByRole(container, 'combobox'))
await type(getByRole(container, 'combobox'), 'singe tête')
await waitFor(() => expect(getAllByRole(container, 'option')).toHaveLength(1))
@ -79,7 +79,7 @@ describe('element tests', () => {
await tick(120)
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(ALL_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await type(getByRole(container, 'combobox'), 'monkey face')
await waitFor(() => expect(getAllByRole(container, 'option')).toHaveLength(1), {
timeout: 2000
@ -91,7 +91,7 @@ describe('element tests', () => {
await tick(120)
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(FR_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await clear(getByRole(container, 'combobox'))
await type(getByRole(container, 'combobox'), 'singe tête')
await waitFor(() => expect(getAllByRole(container, 'option')).toHaveLength(1))

View File

@ -22,7 +22,7 @@ describe('lifecycle', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
document.body.removeChild(picker)
await tick(40)
@ -33,7 +33,7 @@ describe('lifecycle', () => {
// fetch is called once again after re-insertion
expect(fetch.calls().length).toBe(2)
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toEqual({ method: 'HEAD' })
expect(fetch.lastOptions().method).toBe('HEAD')
document.body.removeChild(picker)
await tick(60)
@ -107,7 +107,7 @@ describe('lifecycle', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await expect(() => (
expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
))
@ -135,7 +135,7 @@ describe('lifecycle', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await expect(() => (
expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
))
@ -148,7 +148,7 @@ describe('lifecycle', () => {
expect(fetch.calls().length).toBe(2) // fetch is called again due to re-render
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toEqual({ method: 'HEAD' }) // cached, so does a HEAD
expect(fetch.lastOptions().method).toBe('HEAD') // cached, so does a HEAD
await expect(() => (
expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
))
@ -171,7 +171,7 @@ describe('lifecycle', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
await expect(() => (
expect(getByRole(picker.shadowRoot, 'option', { name: /😀/ })).toBeVisible()
))

View File

@ -25,7 +25,7 @@ describe('properties', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(FR_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(picker.locale).toEqual('fr')
expect(picker.dataSource).toEqual(FR_EMOJI)
@ -46,7 +46,7 @@ describe('properties', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(FR_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(picker.locale).toEqual('en')
expect(picker.dataSource).toEqual(FR_EMOJI)
@ -67,7 +67,7 @@ describe('properties', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(DEFAULT_DATA_SOURCE)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(picker.locale).toEqual('fr')
expect(picker.dataSource).toEqual(DEFAULT_DATA_SOURCE)

View File

@ -39,7 +39,7 @@ describe('upgrade tests', () => {
expect(fetch.calls().length).toBe(1)
expect(fetch.lastUrl()).toBe(FR_EMOJI)
expect(fetch.lastOptions()).toBe(undefined)
expect(fetch.lastOptions().method).toBe(undefined)
expect(getByRole(container, 'button', { name: /Choose a skin tone/ }).innerHTML).toContain('👍')