[Search Sessions] Cancel the previous session

packages/kbn-optimizer/limits.yml

@@ -3,7 +3,7 @@ pageLoadAssetSize:
   alerting: 106936
   apm: 64385
   apmOss: 18996
-  bfetch: 41874
+  bfetch: 51874
   canvas: 1066647
   charts: 195358
   cloud: 21076

src/plugins/data/public/search/session/mocks.ts

@@ -20,6 +20,7 @@ export function getSessionsClientMock(): jest.Mocked<ISessionsClient> {
     extend: jest.fn(),
     delete: jest.fn(),
     rename: jest.fn(),
+    cancel: jest.fn(),
   };
 }
 

src/plugins/data/public/search/session/session_service.test.ts

@@ -59,6 +59,7 @@ describe('Session service', () => {
       id,
       attributes: { ...mockSavedObject.attributes, sessionId: id },
     }));
+    sessionsClient.cancel.mockResolvedValue(undefined);
     sessionService = new SessionService(
       initializerContext,
       () =>
@@ -98,6 +99,15 @@ describe('Session service', () => {
       expect(nowProvider.reset).toHaveBeenCalled();
     });
 
+    it('Calling start twice clears the previous session', async () => {
+      sessionService.start();
+      expect(sessionService.getSessionId()).not.toBeUndefined();
+      expect(nowProvider.set).toHaveBeenCalled();
+      sessionService.start();
+      expect(sessionService.getSessionId()).not.toBeUndefined();
+      expect(sessionsClient.cancel).toHaveBeenCalled();
+    });
+
     it("Can't clear other apps' session", async () => {
       sessionService.start();
       expect(sessionService.getSessionId()).not.toBeUndefined();

src/plugins/data/public/search/session/session_service.ts

@@ -213,6 +213,10 @@ export class SessionService {
    */
   public start() {
     if (!this.currentApp) throw new Error('this.currentApp is missing');
+
+    // cancel previous session if needed
+    this.cleanupPreviousSession();
+
     this.state.transitions.start({ appName: this.currentApp });
     return this.getSessionId()!;
   }
@@ -241,24 +245,34 @@ export class SessionService {
       );
       return;
     }
-
+    this.cleanupPreviousSession();
     this.state.transitions.clear();
     this.searchSessionInfoProvider = undefined;
     this.searchSessionIndicatorUiConfig = undefined;
   }
 
+  private async cleanupPreviousSession() {
+    const { pendingSearches, sessionId, isRestore, isStored } = this.state.get();
+    if (sessionId && !isRestore && !isStored) {
+      pendingSearches.forEach((s) => {
+        s.abort();
+      });
+      await this.sessionsClient.cancel(sessionId).catch(() => {});
+    }
+  }
+
   /**
    * Request a cancellation of on-going search requests within current session
    */
   public async cancel(): Promise<void> {
-    const isStoredSession = this.state.get().isStored;
-    this.state.get().pendingSearches.forEach((s) => {
+    const { pendingSearches, sessionId } = this.state.get();
+    if (!sessionId) return;
+
+    pendingSearches.forEach((s) => {
       s.abort();
     });
+    await this.sessionsClient.cancel(sessionId).catch(() => {});
     this.state.transitions.cancel();
-    if (isStoredSession) {
-      await this.sessionsClient.delete(this.state.get().sessionId!);
-    }
   }
 
   /**

src/plugins/data/public/search/session/sessions_client.ts

@@ -92,6 +92,10 @@ export class SessionsClient {
     });
   }
 
+  public cancel(sessionId: string): Promise<void> {
+    return this.http!.post(`/internal/session/${encodeURIComponent(sessionId)}/cancel`);
+  }
+
   public delete(sessionId: string): Promise<void> {
     return this.http!.delete(`/internal/session/${encodeURIComponent(sessionId)}`);
   }

x-pack/plugins/data_enhanced/server/routes/session.test.ts

@@ -118,6 +118,45 @@ describe('registerSessionRoutes', () => {
     expect(mockContext.search!.cancelSession).toHaveBeenCalledWith(id);
   });
 
+  it('cancel doesnt fail if not found', async () => {
+    const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
+    const params = { id };
+
+    const mockRequest = httpServerMock.createKibanaRequest({ params });
+    const mockResponse = httpServerMock.createResponseFactory();
+
+    const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
+    const [, cancelHandler] = mockRouter.post.mock.calls[PostHandlerIndex.CANCEL];
+
+    mockContext.search!.cancelSession = jest.fn().mockRejectedValue({
+      statusCode: 404,
+    });
+
+    await cancelHandler(mockContext, mockRequest, mockResponse);
+
+    expect(mockContext.search!.cancelSession).toHaveBeenCalledWith(id);
+    expect(mockResponse.ok).toHaveBeenCalledTimes(1);
+  });
+
+  it('cancel fail on other errors', async () => {
+    const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
+    const params = { id };
+
+    const mockRequest = httpServerMock.createKibanaRequest({ params });
+    const mockResponse = httpServerMock.createResponseFactory();
+
+    const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
+    const [, cancelHandler] = mockRouter.post.mock.calls[PostHandlerIndex.CANCEL];
+
+    mockContext.search!.cancelSession = jest.fn().mockRejectedValue({
+      statusCode: 500,
+    });
+
+    await cancelHandler(mockContext, mockRequest, mockResponse).catch(() => {});
+
+    expect(mockResponse.customError).toHaveBeenCalledTimes(1);
+  });
+
   it('delete calls deleteSession with id', async () => {
     const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
     const params = { id };
@@ -133,6 +172,45 @@ describe('registerSessionRoutes', () => {
     expect(mockContext.search!.deleteSession).toHaveBeenCalledWith(id);
   });
 
+  it('delete doesnt fail if not found', async () => {
+    const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
+    const params = { id };
+
+    const mockRequest = httpServerMock.createKibanaRequest({ params });
+    const mockResponse = httpServerMock.createResponseFactory();
+
+    const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
+    const [, deleteHandler] = mockRouter.delete.mock.calls[0];
+
+    mockContext.search!.deleteSession = jest.fn().mockRejectedValue({
+      statusCode: 404,
+    });
+
+    await deleteHandler(mockContext, mockRequest, mockResponse);
+
+    expect(mockContext.search!.deleteSession).toHaveBeenCalledWith(id);
+    expect(mockResponse.ok).toHaveBeenCalledTimes(1);
+  });
+
+  it('delete returns error if another error code occurs', async () => {
+    const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
+    const params = { id };
+
+    const mockRequest = httpServerMock.createKibanaRequest({ params });
+    const mockResponse = httpServerMock.createResponseFactory();
+
+    const mockRouter = mockCoreSetup.http.createRouter.mock.results[0].value;
+    const [, deleteHandler] = mockRouter.delete.mock.calls[0];
+
+    mockContext.search!.deleteSession = jest.fn().mockRejectedValue({
+      statusCode: 500,
+    });
+
+    await deleteHandler(mockContext, mockRequest, mockResponse).catch(() => {});
+
+    expect(mockResponse.customError).toHaveBeenCalledTimes(1);
+  });
+
   it('extend calls extendSession with id', async () => {
     const id = 'd7170a35-7e2c-48d6-8dec-9a056721b489';
     const expires = new Date().toISOString();

x-pack/plugins/data_enhanced/server/routes/session.ts

@@ -151,6 +151,11 @@ export function registerSessionRoutes(router: DataEnhancedPluginRouter, logger:
         return res.ok();
       } catch (e) {
         const err = e.output?.payload || e;
+        // Don't fail on items that were already deleted
+        if (err.statusCode === 404) {
+          logger.debug(`Search session ${id} not found. Ignoring.`);
+          return res.ok();
+        }
         logger.error(err);
         return reportServerError(res, err);
       }
@@ -177,6 +182,11 @@ export function registerSessionRoutes(router: DataEnhancedPluginRouter, logger:
         return res.ok();
       } catch (e) {
         const err = e.output?.payload || e;
+        // Don't fail on items that were already deleted
+        if (err.statusCode === 404) {
+          logger.debug(`Search session ${id} not found. Ignoring.`);
+          return res.ok();
+        }
         logger.error(err);
         return reportServerError(res, err);
       }

x-pack/plugins/data_enhanced/server/search/session/check_running_sessions.ts

@@ -184,9 +184,11 @@ function checkRunningSessionsPage(
                   try {
                     await client.asyncSearch.delete({ id: searchInfo.id });
                   } catch (e) {
-                    logger.error(
-                      `Error while deleting async_search ${searchInfo.id}: ${e.message}`
-                    );
+                    if (e.message !== 'resource_not_found_exception') {
+                      logger.error(
+                        `Error while deleting async_search ${searchInfo.id}: ${e.message}`
+                      );
+                    }
                   }
                 }
               });

x-pack/test/api_integration/apis/search/session.ts

@@ -49,8 +49,8 @@ export default function ({ getService }: FtrProviderContext) {
         await supertest.get(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200);
       });
 
-      it('should fail to delete an unknown session', async () => {
-        await supertest.delete(`/internal/session/123`).set('kbn-xsrf', 'foo').expect(404);
+      it('should NOT fail when deleting an unknown session', async () => {
+        await supertest.delete(`/internal/session/123`).set('kbn-xsrf', 'foo').expect(200);
       });
 
       it('should create and delete a session', async () => {
@@ -487,7 +487,9 @@ export default function ({ getService }: FtrProviderContext) {
           .delete(`/internal/session/${sessionId}`)
           .set('kbn-xsrf', 'foo')
           .auth('other_user', 'password')
-          .expect(404);
+          .expect(200);
+
+        await supertest.get(`/internal/session/${sessionId}`).set('kbn-xsrf', 'foo').expect(200);
       });
 
       it(`should prevent users from cancelling other users' sessions`, async () => {
@@ -508,7 +510,15 @@ export default function ({ getService }: FtrProviderContext) {
           .post(`/internal/session/${sessionId}/cancel`)
           .set('kbn-xsrf', 'foo')
           .auth('other_user', 'password')
-          .expect(404);
+          .expect(200);
+
+        const resp = await supertest
+          .get(`/internal/session/${sessionId}`)
+          .set('kbn-xsrf', 'foo')
+          .expect(200);
+
+        const { status } = resp.body.attributes;
+        expect(status).not.to.equal(SearchSessionStatus.CANCELLED);
       });
 
       it(`should prevent users from extending other users' sessions`, async () => {