From 4d83a7dc5e5488059f360c184f9e429705701e07 Mon Sep 17 00:00:00 2001 From: Miroslav Jancarik Date: Thu, 6 Feb 2025 22:11:19 +0100 Subject: [PATCH] fix: clear ima app instance before next usage --- .changeset/lucky-geckos-bow.md | 5 ++ .../server/lib/factory/IMAInternalFactory.js | 16 +++++++ .../factory/__tests__/serverAppFactorySpec.js | 48 ++++++++++++++++++- packages/server/lib/factory/hooksFactory.js | 16 ++----- .../server/lib/factory/serverAppFactory.js | 7 ++- 5 files changed, 74 insertions(+), 18 deletions(-) create mode 100644 .changeset/lucky-geckos-bow.md diff --git a/.changeset/lucky-geckos-bow.md b/.changeset/lucky-geckos-bow.md new file mode 100644 index 0000000000..53ea0310f4 --- /dev/null +++ b/.changeset/lucky-geckos-bow.md @@ -0,0 +1,5 @@ +--- +"@ima/server": patch +--- + +The ima app instance must be always cleared before storing for next usage. We fixed edge case specific error which could lead to zombie app. diff --git a/packages/server/lib/factory/IMAInternalFactory.js b/packages/server/lib/factory/IMAInternalFactory.js index 165823c877..19ee512fe4 100644 --- a/packages/server/lib/factory/IMAInternalFactory.js +++ b/packages/server/lib/factory/IMAInternalFactory.js @@ -249,8 +249,24 @@ module.exports = function IMAInternalFactory({ return router.route(router.getPath()); } + function _clearApp({ context }) { + if (context.app) { + const { oc } = context.app; + oc.get('$Dispatcher').clear(); + oc.get('$Cache').clear(); + + oc.get('$PageRenderer').unmount(); + oc.get('$PageManager').destroy(); + oc.clear(); + + instanceRecycler.clearInstance(context.app); + context.app = null; + } + } + return { _initApp, + _clearApp, createBootConfig, _importAppMainSync, _createDummyApp, diff --git a/packages/server/lib/factory/__tests__/serverAppFactorySpec.js b/packages/server/lib/factory/__tests__/serverAppFactorySpec.js index e9c6163158..a094d2ac48 100644 --- a/packages/server/lib/factory/__tests__/serverAppFactorySpec.js +++ b/packages/server/lib/factory/__tests__/serverAppFactorySpec.js @@ -75,6 +75,7 @@ describe('Server App Factory', () => { let pageManager = null; let pageStateManager = null; let pageRenderer = null; + let OCCleared = null; let emitter = new Emitter(); let performance = createMonitoring(); @@ -110,6 +111,7 @@ describe('Server App Factory', () => { status: 500, })); languageLoader = jest.fn(); + OCCleared = jest.fn(); applicationFolder = ''; router = toMockedInstance(ServerRouter, { @@ -198,7 +200,9 @@ describe('Server App Factory', () => { return dispatcher; } }, - clear() {}, + clear() { + OCCleared(); + }, }, }; }), @@ -348,6 +352,48 @@ describe('Server App Factory', () => { expect(response.cache).toBeFalsy(); }); + it('should render 500 static page and then 200 ima app page', async () => { + environment.$Server.staticConcurrency = 0; + jest.spyOn(router, 'getCurrentRouteInfo').mockReturnValue({ + route: { + getName() { + return 'home '; + }, + }, + }); + jest + .spyOn(router, 'route') + .mockReturnValue(Promise.reject(new Error('Static 500 error'))); + pageStateManager.getState.mockImplementation(() => { + throw new Error('State error'); + }); + + let response = await serverApp.requestHandlerMiddleware(REQ, RES); + + expect(response.SPA).toBeFalsy(); + expect(response.static).toBeTruthy(); + expect(response.status).toBe(500); + expect(response.content).toBe('read file content'); + expect(response.cache).toBeFalsy(); + expect(OCCleared).toHaveBeenCalledTimes(1); + + jest.resetAllMocks(); + + jest.spyOn(router, 'route').mockReturnValue({ + status: 200, + content: 'app html', + }); + + response = await serverApp.requestHandlerMiddleware(REQ, RES); + + expect(response.SPA).toBeFalsy(); + expect(response.static).toBeFalsy(); + expect(response.status).toBe(200); + expect(response.content).toBe('app html'); + expect(response.cache).toBeFalsy(); + expect(OCCleared).toHaveBeenCalledTimes(1); + }); + it('should render 500 static page for ima app route ERROR which exceeds static thresholds', async () => { environment.$Server.staticConcurrency = 0; jest.spyOn(router, 'getCurrentRouteInfo').mockReturnValue({ diff --git a/packages/server/lib/factory/hooksFactory.js b/packages/server/lib/factory/hooksFactory.js index bb8ad1f3e2..9e0b307e1d 100644 --- a/packages/server/lib/factory/hooksFactory.js +++ b/packages/server/lib/factory/hooksFactory.js @@ -9,6 +9,7 @@ module.exports = function hooksFactory({ renderStaticClientErrorPage, urlParser, _initApp, + _clearApp, _importAppMainSync, _addImaToResponse, _getRouteInfo, @@ -351,19 +352,8 @@ module.exports = function hooksFactory({ res.send(context.response.content); }); - emitter.on(Event.AfterResponse, async ({ context }) => { - if (context.app) { - const { oc } = context.app; - oc.get('$Dispatcher').clear(); - oc.get('$Cache').clear(); - - oc.get('$PageRenderer').unmount(); - oc.get('$PageManager').destroy(); - oc.clear(); - - instanceRecycler.clearInstance(context.app); - context.app = null; - } + emitter.on(Event.AfterResponse, async event => { + _clearApp(event); }); } diff --git a/packages/server/lib/factory/serverAppFactory.js b/packages/server/lib/factory/serverAppFactory.js index 35a76d1074..5a529bba30 100644 --- a/packages/server/lib/factory/serverAppFactory.js +++ b/packages/server/lib/factory/serverAppFactory.js @@ -29,6 +29,7 @@ module.exports = function serverAppFactory({ const { _initApp, + _clearApp, createBootConfig, _importAppMainSync, _addImaToResponse, @@ -75,6 +76,7 @@ module.exports = function serverAppFactory({ renderStaticClientErrorPage, urlParser, _initApp, + _clearApp, _importAppMainSync, _addImaToResponse, _getRouteInfo, @@ -190,10 +192,7 @@ module.exports = function serverAppFactory({ error.cause = event.error; const { res, context } = event; - if (context.app) { - instanceRecycler.clearInstance(context.app); - context.app = null; - } + _clearApp(event); if (res.headersSent) { return context.response;