diff --git a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts index f949d039eb0ce..701792f24bc35 100644 --- a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts +++ b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.test.ts @@ -202,6 +202,7 @@ test('correctly orders plugins and returns exposed values for "setup" and "start setup: Record; start: Record; } + const plugins = new Map([ [ createPlugin('order-4', { required: ['order-2'] }), @@ -754,6 +755,70 @@ describe('stop', () => { jest.useRealTimers(); }); + const nextTick = () => new Promise((resolve) => setImmediate(resolve)); + + it('stops all plugins', async () => { + const [plugin1, plugin2, plugin3] = [ + createPlugin('plugin-1'), + createPlugin('plugin-2'), + createPlugin('plugin-3'), + ].map((plugin, index) => { + jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); + pluginsSystem.addPlugin(plugin); + return plugin; + }); + + const stopSpy1 = jest.spyOn(plugin1, 'stop').mockImplementationOnce(() => Promise.resolve()); + const stopSpy2 = jest.spyOn(plugin2, 'stop').mockImplementationOnce(() => Promise.resolve()); + const stopSpy3 = jest.spyOn(plugin3, 'stop').mockImplementationOnce(() => Promise.resolve()); + + mockCreatePluginSetupContext.mockImplementation(() => ({})); + + await pluginsSystem.setupPlugins(setupDeps); + const stopPromise = pluginsSystem.stopPlugins(); + + await nextTick(); + jest.runAllTimers(); + + await stopPromise; + + expect(stopSpy1).toHaveBeenCalledTimes(1); + expect(stopSpy2).toHaveBeenCalledTimes(1); + expect(stopSpy3).toHaveBeenCalledTimes(1); + }); + + it('stops plugins in the correct order', async () => { + // stop order: 3 => 1 => 2 + const [plugin1, plugin2, plugin3] = [ + createPlugin('plugin-1', { required: ['plugin-2'] }), + createPlugin('plugin-2'), + createPlugin('plugin-3', { required: ['plugin-1', 'plugin-2'] }), + ].map((plugin, index) => { + jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); + pluginsSystem.addPlugin(plugin); + return plugin; + }); + + const stopSpy1 = jest.spyOn(plugin1, 'stop').mockImplementationOnce(() => Promise.resolve()); + const stopSpy2 = jest.spyOn(plugin2, 'stop').mockImplementationOnce(() => Promise.resolve()); + const stopSpy3 = jest.spyOn(plugin3, 'stop').mockImplementationOnce(() => Promise.resolve()); + + mockCreatePluginSetupContext.mockImplementation(() => ({})); + + await pluginsSystem.setupPlugins(setupDeps); + const stopPromise = pluginsSystem.stopPlugins(); + + await nextTick(); + jest.runAllTimers(); + + await stopPromise; + + expect(stopSpy3.mock.invocationCallOrder[0]).toBeLessThan(stopSpy1.mock.invocationCallOrder[0]); + expect(stopSpy1.mock.invocationCallOrder[0]).toBeLessThan(stopSpy2.mock.invocationCallOrder[0]); + }); + it('waits for 30 sec to finish "stop" and move on to the next plugin.', async () => { const [plugin1, plugin2] = [createPlugin('timeout-stop-1'), createPlugin('timeout-stop-2')].map( (plugin, index) => { @@ -774,8 +839,11 @@ describe('stop', () => { await pluginsSystem.setupPlugins(setupDeps); const stopPromise = pluginsSystem.stopPlugins(); + await nextTick(); jest.runAllTimers(); + await stopPromise; + expect(stopSpy1).toHaveBeenCalledTimes(1); expect(stopSpy2).toHaveBeenCalledTimes(1); @@ -785,4 +853,42 @@ describe('stop', () => { ]) ); }); + + it('logs a message if a plugin fails top stop', async () => { + // stop order: 3 => 1 => 2 + const [plugin1, plugin2, plugin3] = [ + createPlugin('plugin-1', { required: ['plugin-2'] }), + createPlugin('plugin-2'), + createPlugin('plugin-3', { required: ['plugin-1', 'plugin-2'] }), + ].map((plugin, index) => { + jest.spyOn(plugin, 'setup').mockResolvedValue(`setup-as-${index}`); + jest.spyOn(plugin, 'start').mockResolvedValue(`started-as-${index}`); + pluginsSystem.addPlugin(plugin); + return plugin; + }); + + const stopSpy1 = jest + .spyOn(plugin1, 'stop') + .mockImplementationOnce(() => Promise.reject('woups')); + const stopSpy2 = jest.spyOn(plugin2, 'stop').mockImplementationOnce(() => Promise.resolve()); + const stopSpy3 = jest.spyOn(plugin3, 'stop').mockImplementationOnce(() => Promise.resolve()); + + mockCreatePluginSetupContext.mockImplementation(() => ({})); + + await pluginsSystem.setupPlugins(setupDeps); + const stopPromise = pluginsSystem.stopPlugins(); + + await nextTick(); + jest.runAllTimers(); + + await stopPromise; + + expect(stopSpy1).toHaveBeenCalledTimes(1); + expect(stopSpy2).toHaveBeenCalledTimes(1); + expect(stopSpy3).toHaveBeenCalledTimes(1); + + expect(loggingSystemMock.collect(logger).warn.flat()).toEqual( + expect.arrayContaining([`"plugin-1" thrown during stop: woups`]) + ); + }); }); diff --git a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts index d7c4df71dd4fc..35e77e84381fd 100644 --- a/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts +++ b/packages/core/plugins/core-plugins-server-internal/src/plugins_system.ts @@ -224,21 +224,38 @@ export class PluginsSystem { this.log.info(`Stopping all plugins.`); - // Stop plugins in the reverse order of when they were set up. - while (this.satupPlugins.length > 0) { - const pluginName = this.satupPlugins.pop()!; - - this.log.debug(`Stopping plugin "${pluginName}"...`); + const reverseDependencyMap = buildReverseDependencyMap(this.plugins); + const pluginStopPromiseMap = new Map>(); + for (let i = this.satupPlugins.length - 1; i > -1; i--) { + const pluginName = this.satupPlugins[i]; + const plugin = this.plugins.get(pluginName)!; + const pluginDependant = reverseDependencyMap.get(pluginName)!; + const dependantPromises = pluginDependant.map( + (dependantName) => pluginStopPromiseMap.get(dependantName)! + ); - const resultMaybe = await withTimeout({ - promise: this.plugins.get(pluginName)!.stop(), - timeoutMs: 30 * Sec, + // Stop plugin as soon as all the dependant plugins are stopped. + const pluginStopPromise = Promise.all(dependantPromises).then(async () => { + this.log.debug(`Stopping plugin "${pluginName}"...`); + + try { + const resultMaybe = await withTimeout({ + promise: plugin.stop(), + timeoutMs: 30 * Sec, + }); + if (resultMaybe?.timedout) { + this.log.warn(`"${pluginName}" plugin didn't stop in 30sec., move on to the next.`); + } + } catch (e) { + this.log.warn(`"${pluginName}" thrown during stop: ${e}`); + } }); - - if (resultMaybe?.timedout) { - this.log.warn(`"${pluginName}" plugin didn't stop in 30sec., move on to the next.`); - } + pluginStopPromiseMap.set(pluginName, pluginStopPromise); } + + await Promise.allSettled(pluginStopPromiseMap.values()); + + this.log.info(`All plugins stopped.`); } /** @@ -334,3 +351,23 @@ export class PluginsSystem { return sortedPluginNames; } } + +const buildReverseDependencyMap = ( + pluginMap: Map +): Map => { + const reverseMap = new Map(); + for (const pluginName of pluginMap.keys()) { + reverseMap.set(pluginName, []); + } + for (const [pluginName, pluginWrapper] of pluginMap.entries()) { + const allDependencies = [...pluginWrapper.requiredPlugins, ...pluginWrapper.optionalPlugins]; + for (const dependency of allDependencies) { + // necessary to evict non-present optional dependency + if (pluginMap.has(dependency)) { + reverseMap.get(dependency)!.push(pluginName); + } + } + reverseMap.set(pluginName, []); + } + return reverseMap; +};