From ecb94d65250cfd8e172b438e7fd27f5faaf8e63f Mon Sep 17 00:00:00 2001 From: Alf Eaton Date: Fri, 11 Oct 2024 10:24:17 +0100 Subject: [PATCH] Improve spellcheck error handling and types (#21012) GitOrigin-RevId: 0227122154f139401793aae2c8ca9c9960fe5e98 --- .../extensions/spelling/index.ts | 11 +- .../extensions/spelling/spellchecker.ts | 23 ++-- .../spelling/spelling-suggestions.tsx | 26 ++-- .../source-editor/hunspell/HunspellManager.ts | 126 +++++++++++++----- .../source-editor/hunspell/hunspell.worker.ts | 2 +- 5 files changed, 136 insertions(+), 52 deletions(-) diff --git a/services/web/frontend/js/features/source-editor/extensions/spelling/index.ts b/services/web/frontend/js/features/source-editor/extensions/spelling/index.ts index 345a357a76..d31befc261 100644 --- a/services/web/frontend/js/features/source-editor/extensions/spelling/index.ts +++ b/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({ ) : 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 diff --git a/services/web/frontend/js/features/source-editor/extensions/spelling/spellchecker.ts b/services/web/frontend/js/features/source-editor/extensions/spelling/spellchecker.ts index e90f98da1b..c36861e8c4 100644 --- a/services/web/frontend/js/features/source-editor/extensions/spelling/spellchecker.ts +++ b/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) { - reject(new Error()) + if ('error' in result) { + 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((resolve, reject) => { if (this.hunspellManager) { this.hunspellManager.send({ type: 'add_word', word }, result => { - if ((result as { error: true }).error) { - reject(new Error()) + if ('error' in result) { + reject(new Error('Error adding word to spellcheck')) } else { resolve() } @@ -176,8 +181,8 @@ export class SpellChecker { return new Promise((resolve, reject) => { if (this.hunspellManager) { this.hunspellManager.send({ type: 'remove_word', word }, result => { - if ((result as { error: true }).error) { - reject(new Error()) + if ('error' in result) { + reject(new Error('Error removing word from spellcheck')) } else { resolve() } diff --git a/services/web/frontend/js/features/source-editor/extensions/spelling/spelling-suggestions.tsx b/services/web/frontend/js/features/source-editor/extensions/spelling/spelling-suggestions.tsx index de53d815f0..fc2a0cd2da 100644 --- a/services/web/frontend/js/features/source-editor/extensions/spelling/spelling-suggestions.tsx +++ b/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 => { - setSuggestions(result.suggestions.slice(0, ITEMS_TO_SHOW)) - setWaiting(false) - sendMB('spelling-suggestion-shown', { - language: spellCheckLanguage, - count: result.suggestions.length, - // word: transaction.state.sliceDoc(mark.from, mark.to), + spellChecker + ?.suggest(word.text) + .then(result => { + setSuggestions(result.suggestions.slice(0, ITEMS_TO_SHOW)) + setWaiting(false) + sendMB('spelling-suggestion-shown', { + 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]) diff --git a/services/web/frontend/js/features/source-editor/hunspell/HunspellManager.ts b/services/web/frontend/js/features/source-editor/hunspell/HunspellManager.ts index 27301330bf..f0b67e89f3 100644 --- a/services/web/frontend/js/features/source-editor/hunspell/HunspellManager.ts +++ b/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 = - | { - id?: string - type: 'spell' - words: string[] - } - | { - id?: string - type: 'suggest' - word: string - } - | { - id?: string - type: 'add_word' - word: string - } - | { - id?: string - type: 'remove_word' - word: string - } - | { - id?: string - type: 'destroy' - } +type SpellMessage = { + type: 'spell' + words: string[] +} + +type SuggestMessage = { + type: 'suggest' + word: string +} + +type AddWordMessage = { + type: 'add_word' + word: string +} + +type RemoveWordMessage = { + type: 'remove_word' + word: string +} + +type DestroyMessage = { + type: 'destroy' +} + +type Message = { id?: string } & ( + | SpellMessage + | SuggestMessage + | AddWordMessage + | RemoveWordMessage + | DestroyMessage +) + +type EmptyResult = Record + +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 void> = new Map() + callbacks: Map = new Map() constructor( private readonly language: string, @@ -64,8 +93,36 @@ export class HunspellManager { }) } - send(message: Message, callback: (value: unknown) => void) { - debugConsole.log(message) + send( + 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 } } } diff --git a/services/web/frontend/js/features/source-editor/hunspell/hunspell.worker.ts b/services/web/frontend/js/features/source-editor/hunspell/hunspell.worker.ts index b8f34efee7..a287bbdb4a 100644 --- a/services/web/frontend/js/features/source-editor/hunspell/hunspell.worker.ts +++ b/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