Improve spellcheck error handling and types (#21012)

GitOrigin-RevId: 0227122154f139401793aae2c8ca9c9960fe5e98

services/web/frontend/js/features/source-editor/extensions/spelling/index.ts

@@ -19,6 +19,7 @@ import { SpellChecker } from './spellchecker'
 import { parserWatcher } from '../wait-for-parser'
 import type { HunspellManager } from '@/features/source-editor/hunspell/HunspellManager'
 import { debugConsole } from '@/utils/debugging'
+import { captureException } from '@/infrastructure/error-reporter'
 
 type Options = {
   spellCheckLanguage?: string
@@ -81,9 +82,15 @@ const spellCheckerField = StateField.define<SpellChecker | null>({
             )
           : null
       } else if (effect.is(addIgnoredWord)) {
-        value?.addWord(effect.value.text).catch(debugConsole.error)
+        value?.addWord(effect.value.text).catch(error => {
+          captureException(error)
+          debugConsole.error(error)
+        })
       } else if (effect.is(removeIgnoredWord)) {
-        value?.removeWord(effect.value.text).catch(debugConsole.error)
+        value?.removeWord(effect.value.text).catch(error => {
+          captureException(error)
+          debugConsole.error(error)
+        })
       }
     }
     return value

services/web/frontend/js/features/source-editor/extensions/spelling/spellchecker.ts

@@ -14,6 +14,7 @@ import {
 import { waitForParser } from '../wait-for-parser'
 import { debugConsole } from '@/utils/debugging'
 import type { HunspellManager } from '../../hunspell/HunspellManager'
+import { captureException } from '@/infrastructure/error-reporter'
 
 /*
  * Spellchecker, handles updates, schedules spelling checks
@@ -120,10 +121,14 @@ export class SpellChecker {
             type: 'spell',
             words: unknownWords.map(word => word.text),
           },
-          (result: any) => {
+          result => {
             if (!signal.aborted) {
-              if (result.error) {
+              if ('error' in result) {
                 debugConsole.error(result.error)
+                captureException(
+                  new Error('Error running spellcheck for word'),
+                  { language: this.language }
+                )
               } else {
                 processResult(result.misspellings)
               }
@@ -148,10 +153,10 @@ export class SpellChecker {
     return new Promise<{ suggestions: string[] }>((resolve, reject) => {
       if (this.hunspellManager) {
         this.hunspellManager.send({ type: 'suggest', word }, result => {
-          if ((result as { error: true }).error) {
+          if ('error' in result) {
-            reject(new Error())
+            reject(new Error('Error finding spelling suggestions for word'))
           } else {
-            resolve(result as { suggestions: string[] })
+            resolve(result)
           }
         })
       }
@@ -162,8 +167,8 @@ export class SpellChecker {
     return new Promise<void>((resolve, reject) => {
       if (this.hunspellManager) {
         this.hunspellManager.send({ type: 'add_word', word }, result => {
-          if ((result as { error: true }).error) {
+          if ('error' in result) {
-            reject(new Error())
+            reject(new Error('Error adding word to spellcheck'))
           } else {
             resolve()
           }
@@ -176,8 +181,8 @@ export class SpellChecker {
     return new Promise<void>((resolve, reject) => {
       if (this.hunspellManager) {
         this.hunspellManager.send({ type: 'remove_word', word }, result => {
-          if ((result as { error: true }).error) {
+          if ('error' in result) {
-            reject(new Error())
+            reject(new Error('Error removing word from spellcheck'))
           } else {
             resolve()
           }

services/web/frontend/js/features/source-editor/extensions/spelling/spelling-suggestions.tsx

@@ -13,6 +13,8 @@ import classnames from 'classnames'
 import { useFeatureFlag } from '@/shared/context/split-test-context'
 import { sendMB } from '@/infrastructure/event-tracking'
 import SpellingSuggestionsFeedback from './spelling-suggestions-feedback'
+import { captureException } from '@/infrastructure/error-reporter'
+import { debugConsole } from '@/utils/debugging'
 
 const ITEMS_TO_SHOW = 8
 
@@ -51,15 +53,23 @@ export const SpellingSuggestions: FC<{
 
   useEffect(() => {
     if (!word.suggestions) {
-      spellChecker?.suggest(word.text).then(result => {
+      spellChecker
-        setSuggestions(result.suggestions.slice(0, ITEMS_TO_SHOW))
+        ?.suggest(word.text)
-        setWaiting(false)
+        .then(result => {
-        sendMB('spelling-suggestion-shown', {
+          setSuggestions(result.suggestions.slice(0, ITEMS_TO_SHOW))
-          language: spellCheckLanguage,
+          setWaiting(false)
-          count: result.suggestions.length,
+          sendMB('spelling-suggestion-shown', {
-          // word: transaction.state.sliceDoc(mark.from, mark.to),
+            language: spellCheckLanguage,
+            count: result.suggestions.length,
+            // word: transaction.state.sliceDoc(mark.from, mark.to),
+          })
+        })
+        .catch(error => {
+          captureException(error, {
+            language: spellCheckLanguage,
+          })
+          debugConsole.error(error)
         })
-      })
     }
   }, [word, spellChecker, spellCheckLanguage])
 

services/web/frontend/js/features/source-editor/hunspell/HunspellManager.ts

@@ -1,32 +1,60 @@
+/* eslint-disable no-dupe-class-members */
+
 import { v4 as uuid } from 'uuid'
 import getMeta from '@/utils/meta'
 import { debugConsole } from '@/utils/debugging'
+import { captureException } from '@/infrastructure/error-reporter'
 
-type Message =
+type SpellMessage = {
-  | {
+  type: 'spell'
-      id?: string
+  words: string[]
-      type: 'spell'
+}
-      words: string[]
+
-    }
+type SuggestMessage = {
-  | {
+  type: 'suggest'
-      id?: string
+  word: string
-      type: 'suggest'
+}
-      word: string
+
-    }
+type AddWordMessage = {
-  | {
+  type: 'add_word'
-      id?: string
+  word: string
-      type: 'add_word'
+}
-      word: string
+
-    }
+type RemoveWordMessage = {
-  | {
+  type: 'remove_word'
-      id?: string
+  word: string
-      type: 'remove_word'
+}
-      word: string
+
-    }
+type DestroyMessage = {
-  | {
+  type: 'destroy'
-      id?: string
+}
-      type: 'destroy'
+
-    }
+type Message = { id?: string } & (
+  | SpellMessage
+  | SuggestMessage
+  | AddWordMessage
+  | RemoveWordMessage
+  | DestroyMessage
+)
+
+type EmptyResult = Record<string, never>
+
+type ErrorResult = {
+  error: true
+}
+
+type SpellResult = {
+  misspellings: { index: number }[]
+}
+
+type SuggestResult = {
+  suggestions: string[]
+}
+
+type ResultCallback =
+  | ((value: SpellResult | ErrorResult) => void)
+  | ((value: SuggestResult | ErrorResult) => void)
+  | ((value: EmptyResult | ErrorResult) => void)
 
 export class HunspellManager {
   baseAssetPath: string
@@ -35,8 +63,9 @@ export class HunspellManager {
   abortController: AbortController | undefined
   listening = false
   loaded = false
+  loadingFailed = false
   pendingMessages: Message[] = []
-  callbacks: Map<string, (value: unknown) => void> = new Map()
+  callbacks: Map<string, ResultCallback> = new Map()
 
   constructor(
     private readonly language: string,
@@ -64,8 +93,36 @@ export class HunspellManager {
     })
   }
 
-  send(message: Message, callback: (value: unknown) => void) {
+  send(
-    debugConsole.log(message)
+    message: AddWordMessage,
+    callback: (value: EmptyResult | ErrorResult) => void
+  ): void
+
+  send(
+    message: RemoveWordMessage,
+    callback: (value: EmptyResult | ErrorResult) => void
+  ): void
+
+  send(
+    message: DestroyMessage,
+    callback: (value: EmptyResult | ErrorResult) => void
+  ): void
+
+  send(
+    message: SuggestMessage,
+    callback: (value: SuggestResult | ErrorResult) => void
+  ): void
+
+  send(
+    message: SpellMessage,
+    callback: (value: SpellResult | ErrorResult) => void
+  ): void
+
+  send(message: Message, callback: ResultCallback): void {
+    if (this.loadingFailed) {
+      return // ignore the message
+    }
+
     if (callback) {
       message.id = uuid()
       this.callbacks.set(message.id, callback)
@@ -80,14 +137,14 @@ export class HunspellManager {
 
   receive(event: MessageEvent) {
     debugConsole.log(event.data)
-    const { id, listening, loaded, ...rest } = event.data
+    const { id, ...rest } = event.data
     if (id) {
       const callback = this.callbacks.get(id)
       if (callback) {
         this.callbacks.delete(id)
         callback(rest)
       }
-    } else if (listening) {
+    } else if (rest.listening) {
       this.listening = true
       this.hunspellWorker.postMessage({
         type: 'init',
@@ -100,9 +157,14 @@ export class HunspellManager {
         this.hunspellWorker.postMessage(message)
         this.pendingMessages.length = 0
       }
-    } else if (loaded) {
+    } else if (rest.loaded) {
       this.loaded = true
-      // TODO: use this to display pending state?
+    } else if (rest.loadingFailed) {
+      captureException(new Error('Spell check loading failed'), {
+        language: this.language,
+      })
+      this.loadingFailed = true
+      this.pendingMessages.length = 0
     }
   }
 }

services/web/frontend/js/features/source-editor/hunspell/hunspell.worker.ts

@@ -171,7 +171,7 @@ self.addEventListener('message', async event => {
         self.postMessage({ loaded: true })
       } catch (error) {
         console.error(error)
-        self.postMessage({ error: true })
+        self.postMessage({ loadingFailed: true })
       }
       break