fix: clear ima app instance before next usage

seznam · Feb 7, 2025 · 2b2878e · 2b2878e
1 parent 719f6d0
commit 2b2878e
Show file tree

Hide file tree

Showing 5 changed files with 74 additions and 18 deletions.
diff --git a/.changeset/lucky-geckos-bow.md 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
@@ -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
@@ -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
@@ -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
@@ -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;