fix: handle AbortError correctly

nolanlawson · Mar 19, 2024 · 517a618 · 517a618
1 parent ec880b4
commit 517a618
Show file tree

Hide file tree

Showing 6 changed files with 75 additions and 51 deletions.
diff --git a/src/database/Database.js b/src/database/Database.js
@@ -10,18 +10,15 @@ import { uniqEmoji } from './utils/uniqEmoji'
 import {
   closeDatabase,
   deleteDatabase,
-  addOnCloseListener,
-  openDatabase
+  initializeDatabase
 } from './databaseLifecycle'
 import {
-  isEmpty, getEmojiByGroup,
+  getEmojiByGroup,
   getEmojiBySearchQuery, getEmojiByShortcode, getEmojiByUnicode,
   get, set, getTopFavoriteEmoji, incrementFavoriteEmojiCount
 } from './idbInterface'
 import { customEmojiIndex } from './customEmojiIndex'
 import { cleanEmoji } from './utils/cleanEmoji'
-import { loadDataForFirstTime, checkForUpdates } from './dataLoading'
-import { abortOpportunity } from './utils/abortSignalUtils.js'
 
 export default class Database {
   constructor ({ dataSource = DEFAULT_DATA_SOURCE, locale = DEFAULT_LOCALE, customEmoji = [] } = {}) {
@@ -37,32 +34,21 @@ export default class Database {
   }
 
   async _init () {
-    const controller = this._controller = new AbortController() // used to cancel inflight requests if necessary
-    const { signal } = controller
-    const db = this._db = await openDatabase(this._dbName)
-    addOnCloseListener(this._dbName, this._clear)
-    /* istanbul ignore else */
-    if (import.meta.env.MODE === 'test') {
-      await abortOpportunity()
-    }
-    if (signal.aborted) {
-      return
-    }
-
-    const dataSource = this.dataSource
-    const empty = await isEmpty(db)
-    /* istanbul ignore else */
-    if (import.meta.env.MODE === 'test') {
-      await abortOpportunity()
-    }
-    if (signal.aborted) {
-      return
-    }
-
-    if (empty) {
-      await loadDataForFirstTime(db, dataSource, signal)
-    } else { // offline-first - do an update asynchronously
-      this._lazyUpdate = checkForUpdates(db, dataSource, signal)
+    try {
+      this._controller = new AbortController() // used to cancel inflight requests if necessary
+      const { db, lazyUpdate } = await initializeDatabase(
+        this._dbName,
+        this.dataSource,
+        this._clear,
+        this._controller.signal
+      )
+      this._db = db
+      this._lazyUpdate = lazyUpdate
+    } catch (err) {
+      if (err.name !== 'AbortError') {
+        throw err
+      }
+      // ignore AbortErrors - we were canceled
     }
   }
 

diff --git a/src/database/dataLoading.js b/src/database/dataLoading.js
@@ -1,7 +1,7 @@
 import { getETag, getETagAndData } from './utils/ajax'
 import { jsonChecksum } from './utils/jsonChecksum'
 import { hasData, loadData } from './idbInterface'
-import { abortOpportunity } from './utils/abortSignalUtils.js'
+import { AbortError, abortOpportunity } from './utils/abortSignalUtils.js'
 
 export async function checkForUpdates (db, dataSource, signal) {
   // just do a simple HEAD request first to see if the eTags match
@@ -10,13 +10,6 @@ export async function checkForUpdates (db, dataSource, signal) {
 
   if (!eTag) { // work around lack of ETag/Access-Control-Expose-Headers
     const eTagAndData = await getETagAndData(dataSource, signal)
-    /* istanbul ignore else */
-    if (import.meta.env.MODE === 'test') {
-      await abortOpportunity()
-    }
-    if (signal.aborted) {
-      return
-    }
 
     eTag = eTagAndData[0]
     emojiData = eTagAndData[1]
@@ -27,7 +20,7 @@ export async function checkForUpdates (db, dataSource, signal) {
         await abortOpportunity()
       }
       if (signal.aborted) {
-        return
+        throw new AbortError()
       }
     }
   }
@@ -37,7 +30,7 @@ export async function checkForUpdates (db, dataSource, signal) {
     await abortOpportunity()
   }
   if (signal.aborted) {
-    return
+    throw new AbortError()
   }
 
   if (doesHaveData) {
@@ -46,13 +39,6 @@ export async function checkForUpdates (db, dataSource, signal) {
     console.log('Database update available')
     if (!emojiData) {
       const eTagAndData = await getETagAndData(dataSource, signal)
-      /* istanbul ignore else */
-      if (import.meta.env.MODE === 'test') {
-        await abortOpportunity()
-      }
-      if (signal.aborted) {
-        return
-      }
 
       emojiData = eTagAndData[1]
     }
@@ -61,6 +47,10 @@ export async function checkForUpdates (db, dataSource, signal) {
 }
 
 export async function loadDataForFirstTime (db, dataSource, signal) {
+  /* istanbul ignore else */
+  if (import.meta.env.MODE === 'test') {
+    await abortOpportunity() // the fetch will error if the signal is aborted
+  }
   let [eTag, emojiData] = await getETagAndData(dataSource, signal)
 
   if (!eTag) {

diff --git a/src/database/databaseLifecycle.js b/src/database/databaseLifecycle.js
@@ -1,5 +1,8 @@
 import { initialMigration } from './migrations'
 import { DB_VERSION_INITIAL, DB_VERSION_CURRENT } from './constants'
+import { AbortError, abortOpportunity } from './utils/abortSignalUtils.js'
+import { isEmpty } from './idbInterface.js'
+import { checkForUpdates, loadDataForFirstTime } from './dataLoading.js'
 
 export const openIndexedDBRequests = {}
 const databaseCache = {}
@@ -105,3 +108,32 @@ export function addOnCloseListener (dbName, listener) {
   }
   listeners.push(listener)
 }
+
+export async function initializeDatabase (dbName, dataSource, onClear, signal) {
+  const db = await openDatabase(dbName)
+  addOnCloseListener(dbName, onClear)
+  /* istanbul ignore else */
+  if (import.meta.env.MODE === 'test') {
+    await abortOpportunity()
+  }
+  if (signal.aborted) {
+    throw new AbortError()
+  }
+
+  const empty = await isEmpty(db)
+  /* istanbul ignore else */
+  if (import.meta.env.MODE === 'test') {
+    await abortOpportunity()
+  }
+  if (signal.aborted) {
+    throw new AbortError()
+  }
+
+  let lazyUpdate
+  if (empty) {
+    await loadDataForFirstTime(db, dataSource, signal)
+  } else { // offline-first - do an update asynchronously
+    lazyUpdate = checkForUpdates(db, dataSource, signal)
+  }
+  return { db, lazyUpdate }
+}
diff --git a/src/database/utils/abortSignalUtils.js b/src/database/utils/abortSignalUtils.js
@@ -7,3 +7,10 @@ export async function abortOpportunity () {
   await Promise.resolve()
   await Promise.resolve()
 }
+
+export class AbortError extends Error {
+  constructor () {
+    super('The operation was aborted')
+    this.name = 'AbortError'
+  }
+}
diff --git a/src/database/utils/ajax.js b/src/database/utils/ajax.js
@@ -1,5 +1,6 @@
 import { warnETag } from './warnETag'
 import { assertEmojiData } from './assertEmojiData'
+import { abortOpportunity } from './abortSignalUtils.js'
 
 function assertStatus (response, dataSource) {
   if (Math.floor(response.status / 100) !== 2) {
@@ -9,6 +10,10 @@ function assertStatus (response, dataSource) {
 
 export async function getETag (dataSource, signal) {
   performance.mark('getETag')
+  /* istanbul ignore else */
+  if (import.meta.env.MODE === 'test') {
+    await abortOpportunity() // the fetch will error if the signal is aborted
+  }
   const response = await fetch(dataSource, { method: 'HEAD', signal })
   assertStatus(response, dataSource)
   const eTag = response.headers.get('etag')
@@ -19,6 +24,10 @@ export async function getETag (dataSource, signal) {
 
 export async function getETagAndData (dataSource, signal) {
   performance.mark('getETagAndData')
+  /* istanbul ignore else */
+  if (import.meta.env.MODE === 'test') {
+    await abortOpportunity() // the fetch will error if the signal is aborted
+  }
   const response = await fetch(dataSource, { signal })
   assertStatus(response, dataSource)
   const eTag = response.headers.get('etag')

diff --git a/test/spec/database/timing.test.js b/test/spec/database/timing.test.js
@@ -68,17 +68,17 @@ describe('database timing tests', () => {
             {
               testName: 'basic',
               dataSource: ALL_EMOJI,
-              maxExpectedAbortOpportunityCount: secondLoad ? 4 : 3
+              maxExpectedAbortOpportunityCount: secondLoad ? (dataChanged ? 6 : 5) : 5
             },
             {
               testName: 'misconfigured etag',
               dataSource: ALL_EMOJI_MISCONFIGURED_ETAG,
-              maxExpectedAbortOpportunityCount: secondLoad ? 4 : 3
+              maxExpectedAbortOpportunityCount: secondLoad ? 5 : 5
             },
             {
               testName: 'no etag',
               dataSource: ALL_EMOJI_NO_ETAG,
-              maxExpectedAbortOpportunityCount: secondLoad ? 5 : 3
+              maxExpectedAbortOpportunityCount: secondLoad ? 6 : 6
             }
           ]
           scenarios.forEach(({ testName, dataSource, maxExpectedAbortOpportunityCount }) => {