From 835fffc7164d475195b7e489539c95c94e03ce05 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Thu, 5 Oct 2023 17:03:39 -0700 Subject: [PATCH 1/2] Cache error and continue when dispose editor --- .../lib/editor/EditorBase.ts | 8 +++- .../lib/editor/createEditorCore.ts | 1 + .../test/editor/EditorTest.ts | 40 ++++++++++++++++++- .../lib/interface/EditorCore.ts | 6 +++ .../lib/interface/EditorOptions.ts | 6 +++ 5 files changed, 59 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..603ce852daa 100644 --- a/packages/roosterjs-editor-core/lib/editor/EditorBase.ts +++ b/packages/roosterjs-editor-core/lib/editor/EditorBase.ts @@ -111,8 +111,14 @@ export class EditorBase= 0; i--) { - core.plugins[i].dispose(); + try { + core.plugins[i].dispose(); + } catch (e) { + // Cache the error and pass it out, then keep going since dispose should always succeed + core.disposeErrorHandler?.(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..30d9b0b072e 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(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..a37e0414f90 100644 --- a/packages/roosterjs-editor-types/lib/interface/EditorCore.ts +++ b/packages/roosterjs-editor-types/lib/interface/EditorCore.ts @@ -84,6 +84,12 @@ 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 error The error object we got + */ + disposeErrorHandler?: (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..a2542f686bc 100644 --- a/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts +++ b/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts @@ -144,4 +144,10 @@ 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 error The error object we got + */ + disposeErrorHandler?: (error: Error) => void; } From 734e832c24f60e75bdefcd7149bd80ab42a21c81 Mon Sep 17 00:00:00 2001 From: Jiuqing Song Date: Thu, 5 Oct 2023 17:06:27 -0700 Subject: [PATCH 2/2] Improve --- packages/roosterjs-editor-core/lib/editor/EditorBase.ts | 6 ++++-- packages/roosterjs-editor-core/test/editor/EditorTest.ts | 2 +- packages/roosterjs-editor-types/lib/interface/EditorCore.ts | 3 ++- .../roosterjs-editor-types/lib/interface/EditorOptions.ts | 3 ++- 4 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/roosterjs-editor-core/lib/editor/EditorBase.ts b/packages/roosterjs-editor-core/lib/editor/EditorBase.ts index 603ce852daa..8712c689075 100644 --- a/packages/roosterjs-editor-core/lib/editor/EditorBase.ts +++ b/packages/roosterjs-editor-core/lib/editor/EditorBase.ts @@ -113,11 +113,13 @@ export class EditorBase= 0; i--) { + const plugin = core.plugins[i]; + try { - core.plugins[i].dispose(); + plugin.dispose(); } catch (e) { // Cache the error and pass it out, then keep going since dispose should always succeed - core.disposeErrorHandler?.(e as Error); + core.disposeErrorHandler?.(plugin, e as Error); } } diff --git a/packages/roosterjs-editor-core/test/editor/EditorTest.ts b/packages/roosterjs-editor-core/test/editor/EditorTest.ts index 30d9b0b072e..cf990c0e170 100644 --- a/packages/roosterjs-editor-core/test/editor/EditorTest.ts +++ b/packages/roosterjs-editor-core/test/editor/EditorTest.ts @@ -544,6 +544,6 @@ describe('Dispose with exception', () => { expect(disposeSpy2).toHaveBeenCalledTimes(1); expect(disposeSpy3).toHaveBeenCalledTimes(1); expect(handlerSpy).toHaveBeenCalledTimes(1); - expect(handlerSpy).toHaveBeenCalledWith(new Error(errorMsg)); + 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 a37e0414f90..bf3d51e9a74 100644 --- a/packages/roosterjs-editor-types/lib/interface/EditorCore.ts +++ b/packages/roosterjs-editor-types/lib/interface/EditorCore.ts @@ -87,9 +87,10 @@ export default interface EditorCore extends PluginState { /** * 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?: (error: Error) => void; + 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 a2542f686bc..226c74f7365 100644 --- a/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts +++ b/packages/roosterjs-editor-types/lib/interface/EditorOptions.ts @@ -147,7 +147,8 @@ export default interface EditorOptions { /** * 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?: (error: Error) => void; + disposeErrorHandler?: (plugin: EditorPlugin, error: Error) => void; }