Add unit tests, log unexpected errors instead of rethrowing

marshallmain · marshallmain · commit d3f099aa23c4 · 2021-07-06T15:48:55.000-07:00
diff --git a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/index.ts b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/index.ts
@@ -19,6 +19,7 @@ import { defaultLifecyclePolicy } from '../../common/assets/lifecycle_policies/d
 import { ClusterPutComponentTemplateBody, PutIndexTemplateRequest } from '../../common/types';
 import { RuleDataClient } from '../rule_data_client';
 import { RuleDataWriteDisabledError } from './errors';
+import { incrementIndexName } from './utils';
 
 const BOOTSTRAP_TIMEOUT = 60000;
 
@@ -51,12 +52,6 @@ function createSignal() {
   return { wait, complete, isReady: () => ready };
 }
 
-function incrementIndexName(oldIndex: string) {
-  const baseIndexString = oldIndex.slice(0, -6);
-  const newIndexNumber = Number(oldIndex.slice(-6)) + 1;
-  return baseIndexString + String(newIndexNumber).padStart(6, '0');
-}
-
 export class RuleDataPluginService {
   signal = createSignal();
 
@@ -150,16 +145,22 @@ export class RuleDataPluginService {
       return;
     } catch (err) {
       if (err.meta?.body?.error?.type !== 'illegal_argument_exception') {
-        throw err;
+        this.options.logger.error(`Failed to PUT mapping for alias ${alias}: ${err.message}`);
+        return;
+      }
+      const newIndexName = incrementIndexName(index);
+      if (newIndexName == null) {
+        this.options.logger.error(`Failed to increment write index name for alias: ${alias}`);
+        return;
       }
       try {
         await clusterClient.indices.rollover({
           alias,
-          new_index: incrementIndexName(index),
+          new_index: newIndexName,
         });
       } catch (e) {
         if (e?.meta?.body?.error?.type !== 'resource_already_exists_exception') {
-          throw e;
+          this.options.logger.error(`Failed to rollover index for alias ${alias}: ${e.message}`);
         }
       }
     }
diff --git a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/utils.test.ts b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/utils.test.ts
@@ -0,0 +1,28 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+import { incrementIndexName } from './utils';
+
+describe('incrementIndexName', () => {
+  it('should increment 000001 to 000002', () => {
+    const oldIndex = '.alerts-mock-000001';
+    const newIndex = incrementIndexName(oldIndex);
+    expect(newIndex).toEqual('.alerts-mock-000002');
+  });
+
+  it('should increment 000010 to 000011', () => {
+    const oldIndex = '.alerts-mock-000010';
+    const newIndex = incrementIndexName(oldIndex);
+    expect(newIndex).toEqual('.alerts-mock-000011');
+  });
+
+  it('should return undefined if oldIndex does not end in a number', () => {
+    const oldIndex = '.alerts-mock-string';
+    const newIndex = incrementIndexName(oldIndex);
+    expect(newIndex).toEqual(undefined);
+  });
+});
diff --git a/x-pack/plugins/rule_registry/server/rule_data_plugin_service/utils.ts b/x-pack/plugins/rule_registry/server/rule_data_plugin_service/utils.ts
@@ -0,0 +1,15 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License
+ * 2.0; you may not use this file except in compliance with the Elastic License
+ * 2.0.
+ */
+
+export function incrementIndexName(oldIndex: string) {
+  const baseIndexString = oldIndex.slice(0, -6);
+  const newIndexNumber = Number(oldIndex.slice(-6)) + 1;
+  if (isNaN(newIndexNumber)) {
+    return undefined;
+  }
+  return baseIndexString + String(newIndexNumber).padStart(6, '0');
+}
