From 565ac17c33deac4a0d59093abff72616f720c664 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Fri, 6 Oct 2023 13:32:35 -0700 Subject: [PATCH] Catch error and continue when dispose editor (#2129) * Cache error and continue when dispose editor * Improve --- .../lib/editor/EditorBase.ts | 10 ++++- .../lib/editor/createEditorCore.ts | 1 + .../test/editor/EditorTest.ts | 40 ++++++++++++++++++- .../lib/interface/EditorCore.ts | 7 ++++ .../lib/interface/EditorOptions.ts | 7 ++++ 5 files changed, 63 insertions(+), 2 deletions(-) diff --git a/packages/roosterjs-editor-core/lib/editor/EditorBase.ts b/packages/roosterjs-editor-core/lib/editor/EditorBase.ts index 34914a190e8..8712c689075 100644 --- a/packages/roosterjs-editor-core/lib/editor/EditorBase.ts +++ b/packages/roosterjs-editor-core/lib/editor/EditorBase.ts @@ -111,8 +111,16 @@ export class EditorBase= 0; i--) { - core.plugins[i].dispose(); + const plugin = core.plugins[i]; + + try { + plugin.dispose(); + } catch (e) { + // Cache the error and pass it out, then keep going since dispose should always succeed + core.disposeErrorHandler?.(plugin, e as Error); + } } core.darkColorHandler.reset(); diff --git a/packages/roosterjs-editor-core/lib/editor/createEditorCore.ts b/packages/roosterjs-editor-core/lib/editor/createEditorCore.ts index 28cbac2817d..7bdaca0b31e 100644 --- a/packages/roosterjs-editor-core/lib/editor/createEditorCore.ts +++ b/packages/roosterjs-editor-core/lib/editor/createEditorCore.ts @@ -52,6 +52,7 @@ export const createEditorCore: CoreCreator = (content getVisibleViewport, imageSelectionBorderColor: options.imageSelectionBorderColor, darkColorHandler: new DarkColorHandlerImpl(contentDiv, pluginState.lifecycle.getDarkColor), + disposeErrorHandler: options.disposeErrorHandler, }; return core; diff --git a/packages/roosterjs-editor-core/test/editor/EditorTest.ts b/packages/roosterjs-editor-core/test/editor/EditorTest.ts index ccbcfa82e8e..cf990c0e170 100644 --- a/packages/roosterjs-editor-core/test/editor/EditorTest.ts +++ b/packages/roosterjs-editor-core/test/editor/EditorTest.ts @@ -1,6 +1,7 @@ import * as getSelectionRange from '../../lib/coreApi/getSelectionRange'; import * as TestHelper from '../TestHelper'; -import { ContentPosition, IEditor } from 'roosterjs-editor-types'; +import Editor from '../../lib/editor/Editor'; +import { ContentPosition, EditorPlugin, IEditor } from 'roosterjs-editor-types'; let editor: IEditor; let testID = 'EditorTest'; @@ -509,3 +510,40 @@ describe('Editor getCustomData()', () => { expect(objCount).toBe(0); }); }); + +describe('Dispose with exception', () => { + it('handle exception when dispose', () => { + const errorMsg = 'Test error'; + const disposeSpy1 = jasmine.createSpy('dispose1'); + const disposeSpy2 = jasmine.createSpy('dispose2').and.throwError(errorMsg); + const disposeSpy3 = jasmine.createSpy('dispose3'); + const handlerSpy = jasmine.createSpy('disposeErrorhandler'); + + const plugin1 = ({ + initialize: () => {}, + dispose: disposeSpy1, + } as any) as EditorPlugin; + const plugin2 = ({ + initialize: () => {}, + dispose: disposeSpy2, + } as any) as EditorPlugin; + const plugin3 = ({ + initialize: () => {}, + dispose: disposeSpy3, + } as any) as EditorPlugin; + + const div = document.createElement('div'); + const editor = new Editor(div, { + plugins: [plugin1, plugin2, plugin3], + disposeErrorHandler: handlerSpy, + }); + + editor.dispose(); + + expect(disposeSpy1).toHaveBeenCalledTimes(1); + expect(disposeSpy2).toHaveBeenCalledTimes(1); + expect(disposeSpy3).toHaveBeenCalledTimes(1); + expect(handlerSpy).toHaveBeenCalledTimes(1); + expect(handlerSpy).toHaveBeenCalledWith(plugin2, new Error(errorMsg)); + }); +}); diff --git a/packages/roosterjs-editor-types/lib/interface/EditorCore.ts b/packages/roosterjs-editor-types/lib/interface/EditorCore.ts index f19efad39f0..bf3d51e9a74 100644 --- a/packages/roosterjs-editor-types/lib/interface/EditorCore.ts +++ b/packages/roosterjs-editor-types/lib/interface/EditorCore.ts @@ -84,6 +84,13 @@ export default interface EditorCore extends PluginState { * If keep it null, editor will still use original dataset-based dark mode solution. */ darkColorHandler: DarkColorHandler; + + /** + * A callback to be invoked when any exception is thrown during disposing editor + * @param plugin The plugin that causes exception + * @param error The error object we got + */ + disposeErrorHandler?: (plugin: EditorPlugin, error: Error) => void; } /** diff --git a/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts b/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts index 9e49ed9bb27..226c74f7365 100644 --- a/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts +++ b/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts @@ -144,4 +144,11 @@ export default interface EditorOptions { * Color of the border of a selectedImage. Default color: '#DB626C' */ imageSelectionBorderColor?: string; + + /** + * A callback to be invoked when any exception is thrown during disposing editor + * @param plugin The plugin that causes exception + * @param error The error object we got + */ + disposeErrorHandler?: (plugin: EditorPlugin, error: Error) => void; }