Distinguish not found SO errors for TM metrics (#174840)

Resolves: #174749 ## To verify: Run Kibana and create a rule with actions. Replace one of the getDecryptedAsInternalUser's (e.g. [getActionInfo](https://github.com/elastic/kibana/pull/174840/files#diff-9a1c0e513210895d1de20260558acd5fadacf4b8331879fba6f37a3e96375fabR473)) id input with a random number and check for task_run user_errors on http://localhost:5601/api/task_manager/metrics?reset=false
elastic · Jan 22, 2024 · d50ee37 · d50ee37
1 parent 4ee5e01
commit d50ee37
Show file tree

Hide file tree

Showing 8 changed files with 139 additions and 27 deletions.
diff --git a/x-pack/plugins/actions/server/lib/action_executor.test.ts b/x-pack/plugins/actions/server/lib/action_executor.test.ts
@@ -5,7 +5,7 @@
  * 2.0.
  */
 
-import { KibanaRequest } from '@kbn/core/server';
+import { KibanaRequest, SavedObjectsErrorHelpers } from '@kbn/core/server';
 import { schema } from '@kbn/config-schema';
 import { ActionExecutor } from './action_executor';
 import { actionTypeRegistryMock } from '../action_type_registry.mock';
@@ -1275,6 +1275,19 @@ test('throws an error when failing to load action through savedObjectsClient', a
   }
 });
 
+test('throws a USER error when the action SO is not found', async () => {
+  encryptedSavedObjectsClient.getDecryptedAsInternalUser.mockRejectedValueOnce(
+    SavedObjectsErrorHelpers.createGenericNotFoundError()
+  );
+
+  try {
+    await actionExecutor.execute(executeParams);
+  } catch (e) {
+    expect(e.message).toBe('Not Found');
+    expect(getErrorSource(e)).toBe(TaskErrorSource.USER);
+  }
+});
+
 test('throws an error if actionType is not enabled', async () => {
   const actionType: jest.Mocked<ActionType> = {
     id: 'test',

diff --git a/x-pack/plugins/actions/server/lib/action_executor.ts b/x-pack/plugins/actions/server/lib/action_executor.ts
@@ -6,7 +6,7 @@
  */
 
 import type { PublicMethodsOf } from '@kbn/utility-types';
-import { KibanaRequest, Logger } from '@kbn/core/server';
+import { KibanaRequest, Logger, SavedObjectsErrorHelpers } from '@kbn/core/server';
 import { cloneDeep } from 'lodash';
 import { set } from '@kbn/safer-lodash-set';
 import { withSpan } from '@kbn/apm-utils';
@@ -487,6 +487,9 @@ export class ActionExecutor {
         rawAction: rawAction.attributes,
       };
     } catch (e) {
+      if (SavedObjectsErrorHelpers.isNotFoundError(e)) {
+        throw createTaskRunError(e, TaskErrorSource.USER);
+      }
       throw createTaskRunError(e, TaskErrorSource.FRAMEWORK);
     }
   }

diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.test.ts
@@ -30,6 +30,7 @@ import {
   isUnrecoverableError,
 } from '@kbn/task-manager-plugin/server/task_running';
 import { CoreKibanaRequest } from '@kbn/core-http-router-server-internal';
+import { SavedObjectsErrorHelpers } from '@kbn/core-saved-objects-server';
 
 const executeParamsFields = [
   'actionId',
@@ -947,6 +948,25 @@ describe('Task Runner Factory', () => {
     }
   });
 
+  test(`Should return USER error for a "not found SO"`, async () => {
+    const taskRunner = taskRunnerFactory.create({
+      taskInstance: {
+        ...mockedTaskInstance,
+        attempts: 0,
+      },
+    });
+
+    mockedEncryptedSavedObjectsClient.getDecryptedAsInternalUser.mockRejectedValue(
+      SavedObjectsErrorHelpers.createGenericNotFoundError()
+    );
+
+    try {
+      await taskRunner.run();
+    } catch (e) {
+      expect(getErrorSource(e)).toBe(TaskErrorSource.USER);
+    }
+  });
+
   test('will rethrow the error if the error is thrown instead of returned', async () => {
     const taskRunner = taskRunnerFactory.create({
       taskInstance: {

diff --git a/x-pack/plugins/actions/server/lib/task_runner_factory.ts b/x-pack/plugins/actions/server/lib/task_runner_factory.ts
@@ -17,6 +17,7 @@ import {
   Logger,
   SavedObject,
   SavedObjectReference,
+  SavedObjectsErrorHelpers,
 } from '@kbn/core/server';
 import {
   createTaskRunError,
@@ -287,28 +288,35 @@ async function getActionTaskParams(
   const { spaceId } = executorParams;
   const namespace = spaceIdToNamespace(spaceId);
   if (isPersistedActionTask(executorParams)) {
-    const actionTask =
-      await encryptedSavedObjectsClient.getDecryptedAsInternalUser<ActionTaskParams>(
-        ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE,
-        executorParams.actionTaskParamsId,
-        { namespace }
-      );
-    const {
-      attributes: { relatedSavedObjects },
-      references,
-    } = actionTask;
-
-    const { actionId, relatedSavedObjects: injectedRelatedSavedObjects } =
-      injectSavedObjectReferences(references, relatedSavedObjects as RelatedSavedObjects);
-
-    return {
-      ...actionTask,
-      attributes: {
-        ...actionTask.attributes,
-        ...(actionId ? { actionId } : {}),
-        ...(relatedSavedObjects ? { relatedSavedObjects: injectedRelatedSavedObjects } : {}),
-      },
-    };
+    try {
+      const actionTask =
+        await encryptedSavedObjectsClient.getDecryptedAsInternalUser<ActionTaskParams>(
+          ACTION_TASK_PARAMS_SAVED_OBJECT_TYPE,
+          executorParams.actionTaskParamsId,
+          { namespace }
+        );
+      const {
+        attributes: { relatedSavedObjects },
+        references,
+      } = actionTask;
+
+      const { actionId, relatedSavedObjects: injectedRelatedSavedObjects } =
+        injectSavedObjectReferences(references, relatedSavedObjects as RelatedSavedObjects);
+
+      return {
+        ...actionTask,
+        attributes: {
+          ...actionTask.attributes,
+          ...(actionId ? { actionId } : {}),
+          ...(relatedSavedObjects ? { relatedSavedObjects: injectedRelatedSavedObjects } : {}),
+        },
+      };
+    } catch (e) {
+      if (SavedObjectsErrorHelpers.isNotFoundError(e)) {
+        throw createTaskRunError(e, TaskErrorSource.USER);
+      }
+      throw createTaskRunError(e, TaskErrorSource.FRAMEWORK);
+    }
   } else {
     return { attributes: executorParams.taskParams, references: executorParams.references ?? [] };
   }

diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.test.ts
@@ -27,7 +27,7 @@ import {
 import { RuleRunMetricsStore } from '../lib/rule_run_metrics_store';
 import { alertingEventLoggerMock } from '../lib/alerting_event_logger/alerting_event_logger.mock';
 import { TaskRunnerContext } from './task_runner_factory';
-import { ConcreteTaskInstance } from '@kbn/task-manager-plugin/server';
+import { ConcreteTaskInstance, TaskErrorSource } from '@kbn/task-manager-plugin/server';
 import { Alert } from '../alert';
 import { AlertInstanceState, AlertInstanceContext, RuleNotifyWhen } from '../../common';
 import { asSavedObjectExecutionSource } from '@kbn/actions-plugin/server';
@@ -37,6 +37,7 @@ import { schema } from '@kbn/config-schema';
 import { alertsClientMock } from '../alerts_client/alerts_client.mock';
 import { ExecutionResponseType } from '@kbn/actions-plugin/server/create_execute_function';
 import { RULE_SAVED_OBJECT_TYPE } from '../saved_objects';
+import { getErrorSource } from '@kbn/task-manager-plugin/server/task_running';
 
 jest.mock('./inject_action_params', () => ({
   injectActionParams: jest.fn(),
@@ -425,6 +426,48 @@ describe('Execution Handler', () => {
     expect(actionsClient.bulkEnqueueExecution).toHaveBeenCalledTimes(1);
   });
 
+  test('should throw a USER error when a connector is not found', async () => {
+    actionsClient.bulkEnqueueExecution.mockRejectedValue({
+      statusCode: 404,
+      message: 'Not Found',
+    });
+    const actions = [
+      {
+        id: '1',
+        group: 'default',
+        actionTypeId: 'test2',
+        params: {
+          foo: true,
+          contextVal: 'My other {{context.value}} goes here',
+          stateVal: 'My other {{state.value}} goes here',
+        },
+      },
+    ];
+    const executionHandler = new ExecutionHandler(
+      generateExecutionParams({
+        ...defaultExecutionParams,
+        taskRunnerContext: {
+          ...defaultExecutionParams.taskRunnerContext,
+          actionsConfigMap: {
+            default: {
+              max: 2,
+            },
+          },
+        },
+        rule: {
+          ...defaultExecutionParams.rule,
+          actions,
+        },
+      })
+    );
+
+    try {
+      await executionHandler.run(generateAlert({ id: 2, state: { value: 'state-val' } }));
+    } catch (err) {
+      expect(getErrorSource(err)).toBe(TaskErrorSource.USER);
+    }
+  });
+
   test('limits actionsPlugin.execute per action group', async () => {
     const executionHandler = new ExecutionHandler(generateExecutionParams());
     await executionHandler.run(generateAlert({ id: 2, group: 'other-group' }));

diff --git a/x-pack/plugins/alerting/server/task_runner/execution_handler.ts b/x-pack/plugins/alerting/server/task_runner/execution_handler.ts
@@ -368,6 +368,9 @@ export class ExecutionHandler<
           try {
             enqueueResponse = await this.actionsClient!.bulkEnqueueExecution(c);
           } catch (e) {
+            if (e.statusCode === 404) {
+              throw createTaskRunError(e, TaskErrorSource.USER);
+            }
             throw createTaskRunError(e, TaskErrorSource.FRAMEWORK);
           }
           if (enqueueResponse.errors) {

diff --git a/x-pack/plugins/alerting/server/task_runner/rule_loader.test.ts b/x-pack/plugins/alerting/server/task_runner/rule_loader.test.ts
@@ -6,7 +6,7 @@
  */
 
 import { encryptedSavedObjectsMock } from '@kbn/encrypted-saved-objects-plugin/server/mocks';
-import { CoreKibanaRequest } from '@kbn/core/server';
+import { CoreKibanaRequest, SavedObjectsErrorHelpers } from '@kbn/core/server';
 import { schema } from '@kbn/config-schema';
 
 import { getRuleAttributes, getFakeKibanaRequest, validateRule } from './rule_loader';
@@ -240,6 +240,19 @@ describe('rule_loader', () => {
         expect(getErrorSource(e)).toBe(TaskErrorSource.FRAMEWORK);
       }
     });
+
+    test('returns USER error for a "not found SO"', async () => {
+      encryptedSavedObjects.getDecryptedAsInternalUser.mockRejectedValue(
+        SavedObjectsErrorHelpers.createGenericNotFoundError()
+      );
+
+      try {
+        await getRuleAttributes(context, ruleId, spaceId);
+      } catch (e) {
+        expect(e.message).toMatch('Not Found');
+        expect(getErrorSource(e)).toBe(TaskErrorSource.USER);
+      }
+    });
   });
 
   describe('getFakeKibanaRequest()', () => {

diff --git a/x-pack/plugins/alerting/server/task_runner/rule_loader.ts b/x-pack/plugins/alerting/server/task_runner/rule_loader.ts
@@ -6,7 +6,13 @@
  */
 
 import { addSpaceIdToPath } from '@kbn/spaces-plugin/server';
-import { CoreKibanaRequest, FakeRawRequest, Headers, SavedObject } from '@kbn/core/server';
+import {
+  CoreKibanaRequest,
+  FakeRawRequest,
+  Headers,
+  SavedObject,
+  SavedObjectsErrorHelpers,
+} from '@kbn/core/server';
 import { PublicMethodsOf } from '@kbn/utility-types';
 import {
   LoadedIndirectParams,
@@ -134,6 +140,9 @@ export async function getRuleAttributes<Params extends RuleTypeParams>(
       { namespace }
     );
   } catch (e) {
+    if (SavedObjectsErrorHelpers.isNotFoundError(e)) {
+      throw createTaskRunError(e, TaskErrorSource.USER);
+    }
     throw createTaskRunError(e, TaskErrorSource.FRAMEWORK);
   }