[Code] fixed the issue that the repository can not be deleted in some…

… cases. (#42841)

* [Code] fixed the issue that the repository can not be deleted in some
cases.

* [Code] catch the rejected exception while cancelling the job, and added
a unit test for CancellationService when the promise of the job get
rejected.

* [Code] refined tests for CancellationService by deferred completion of
the promise.

x-pack/legacy/plugins/code/server/queue/cancellation_service.test.ts

@@ -25,12 +25,46 @@ test('Register and cancel cancellation token', async () => {
   const cancelSpy = sinon.spy();
   token.cancel = cancelSpy;
 
-  await service.registerCancelableIndexJob(
-    repoUri,
-    token as CancellationToken,
-    Promise.resolve('resolved')
-  );
-  await service.cancelIndexJob(repoUri);
+  // create a promise and defer its fulfillment
+  let promiseResolve: () => void = () => {};
+  const promise = new Promise(resolve => {
+    promiseResolve = resolve;
+  });
+  await service.registerCancelableIndexJob(repoUri, token as CancellationToken, promise);
+  // do not wait on the promise, or there will be a dead lock
+  const cancelPromise = service.cancelIndexJob(repoUri);
+  // resolve the promise now
+  promiseResolve();
+
+  await cancelPromise;
+
+  expect(cancelSpy.calledOnce).toBeTruthy();
+});
+
+test('Register and cancel cancellation token while an exception is thrown from the job', async () => {
+  const repoUri = 'github.com/elastic/code';
+  const service = new CancellationSerivce();
+  const token = {
+    cancel: (): void => {
+      return;
+    },
+  };
+  const cancelSpy = sinon.spy();
+  token.cancel = cancelSpy;
+
+  // create a promise and defer its rejection
+  let promiseReject: () => void = () => {};
+  const promise = new Promise((resolve, reject) => {
+    promiseReject = reject;
+  });
+  await service.registerCancelableIndexJob(repoUri, token as CancellationToken, promise);
+  // expect no exceptions are thrown when cancelling the job
+  // do not wait on the promise, or there will be a dead lock
+  const cancelPromise = service.cancelIndexJob(repoUri);
+  // reject the promise now
+  promiseReject();
+
+  await cancelPromise;
 
   expect(cancelSpy.calledOnce).toBeTruthy();
 });

x-pack/legacy/plugins/code/server/queue/cancellation_service.ts

@@ -68,6 +68,10 @@ export class CancellationSerivce {
     // Try to cancel the job first.
     await this.cancelJob(jobMap, repoUri);
     jobMap.set(repoUri, { token, jobPromise });
+    // remove the record from the cancellation service when the promise is fulfilled or rejected.
+    jobPromise.finally(() => {
+      jobMap.delete(repoUri);
+    });
   }
 
   private async cancelJob(jobMap: Map<RepositoryUri, CancellableJob>, repoUri: RepositoryUri) {
@@ -77,9 +81,12 @@ export class CancellationSerivce {
       // 1. Use the cancellation token to pass cancel message to job
       token.cancel();
       // 2. waiting on the actual job promise to be resolved
-      await jobPromise;
-      // 3. remove the record from the cancellation service
-      jobMap.delete(repoUri);
+      try {
+        await jobPromise;
+      } catch (e) {
+        // the exception from the job also indicates the job is finished, and it should be the duty of the worker for
+        // the job to handle it, so it's safe to just ignore the exception here
+      }
     }
   }
 }

x-pack/legacy/plugins/code/server/routes/repository.ts

@@ -9,7 +9,7 @@ import Boom from 'boom';
 import { RequestFacade, ResponseToolkitFacade } from '../..';
 import { validateGitUrl } from '../../common/git_url_utils';
 import { RepositoryUtils } from '../../common/repository_utils';
-import { RepositoryConfig, RepositoryUri } from '../../model';
+import { RepositoryConfig, RepositoryUri, WorkerReservedProgress } from '../../model';
 import { RepositoryIndexInitializer, RepositoryIndexInitializerFactory } from '../indexer';
 import { Logger } from '../log';
 import { RepositoryConfigController } from '../repository_config_controller';
@@ -108,10 +108,13 @@ export function repositoryRoute(
         // Check if the repository delete status already exists. If so, we should ignore this
         // request.
         try {
-          await repoObjectClient.getRepositoryDeleteStatus(repoUri);
-          const msg = `Repository ${repoUri} is already in delete.`;
-          log.info(msg);
-          return h.response(msg).code(304); // Not Modified
+          const status = await repoObjectClient.getRepositoryDeleteStatus(repoUri);
+          // if the delete status is an ERROR, we can give it another try
+          if (status.progress !== WorkerReservedProgress.ERROR) {
+            const msg = `Repository ${repoUri} is already in delete.`;
+            log.info(msg);
+            return h.response(msg).code(304); // Not Modified
+          }
         } catch (error) {
           // Do nothing here since this error is expected.
           log.info(`Repository ${repoUri} delete status does not exist. Go ahead with delete.`);