Changes x-pack action executor to return result. (#39820) (#40053)

* Changes x-pack action executor to return result.

Previously, the result was never captured.  It's now captured, and
returned if it's JSONable.  If not JSONable a `{succcess: true}`
object is returned with a 200 response (used to be 204).

Also added logging for action execution calls, in lieu of having an
action history available.

Drive-by:

- `services.log` was set to the unbound Hapi server.log method, so
  places where it was set were changed to bind it to the server.

x-pack/legacy/plugins/actions/server/init.ts

@@ -43,7 +43,7 @@ export function init(server: Legacy.Server) {
       getBasePath: () => basePath,
     };
     return {
-      log: server.log,
+      log: server.log.bind(server),
       callCluster: callWithInternalUser,
       savedObjectsClient: server.savedObjects.getScopedSavedObjectsClient(fakeRequest),
       ...overwrites,

x-pack/legacy/plugins/actions/server/lib/execute.ts

@@ -37,9 +37,43 @@ export async function execute({
   };
   const validatedConfig = validateActionTypeConfig(actionType, mergedActionTypeConfig);
   const validatedParams = validateActionTypeParams(actionType, params);
-  await actionType.executor({
-    services,
-    config: validatedConfig,
-    params: validatedParams,
-  });
+
+  let result;
+  let error;
+  try {
+    result = await actionType.executor({
+      services,
+      config: validatedConfig,
+      params: validatedParams,
+    });
+  } catch (err) {
+    error = err;
+  }
+
+  const { actionTypeId, description } = action.attributes;
+  const actionLabel = `${actionId} - ${actionTypeId} - ${description}`;
+
+  if (error != null) {
+    services.log(
+      ['warning', 'x-pack', 'actions'],
+      `action executed unsuccessfully: ${actionLabel} - ${error.message}`
+    );
+    throw error;
+  }
+
+  services.log(['debug', 'x-pack', 'actions'], `action executed successfully: ${actionLabel}`);
+
+  // return result if it's JSONable, otherwise a simple success object
+  const simpleResult = { status: 'ok' };
+
+  if (result == null || typeof result !== 'object') {
+    return simpleResult;
+  }
+
+  try {
+    JSON.stringify(result);
+    return result;
+  } catch (err) {
+    return simpleResult;
+  }
 }

x-pack/legacy/plugins/actions/server/routes/fire.test.ts

@@ -39,8 +39,8 @@ it('fires an action with proper parameters', async () => {
   execute.mockResolvedValueOnce({ success: true });
 
   const { payload, statusCode } = await server.inject(request);
-  expect(statusCode).toBe(204);
-  expect(payload).toBe('');
+  expect(statusCode).toBe(200);
+  expect(payload).toBe('{"success":true}');
 
   expect(savedObjectsClient.get).toHaveBeenCalledTimes(1);
   expect(savedObjectsClient.get.mock.calls[0]).toMatchInlineSnapshot(`

x-pack/legacy/plugins/actions/server/routes/fire.ts

@@ -55,15 +55,15 @@ export function fireRoute({ server, actionTypeRegistry, getServices }: FireRoute
       const savedObjectsClient = request.getSavedObjectsClient();
       // Ensure user can read the action and has access to it
       await savedObjectsClient.get('action', id);
-      await execute({
+      const result = await execute({
         params,
         actionTypeRegistry,
         actionId: id,
         namespace: namespace === 'default' ? undefined : namespace,
         services: getServices(request.getBasePath(), { savedObjectsClient }),
         encryptedSavedObjectsPlugin: server.plugins.encrypted_saved_objects!,
       });
-      return h.response();
+      return result;
     },
   });
 }

x-pack/legacy/plugins/alerting/server/init.ts

@@ -29,7 +29,7 @@ export function init(server: Legacy.Server) {
       getBasePath: () => basePath,
     };
     return {
-      log: server.log,
+      log: server.log.bind(server),
       callCluster: callWithInternalUser,
       savedObjectsClient: server.savedObjects.getScopedSavedObjectsClient(fakeRequest),
     };
@@ -56,7 +56,7 @@ export function init(server: Legacy.Server) {
     const request = this;
     const savedObjectsClient = request.getSavedObjectsClient();
     const alertsClient = new AlertsClient({
-      log: server.log,
+      log: server.log.bind(server),
       savedObjectsClient,
       alertTypeRegistry,
       taskManager: taskManager!,

x-pack/test/api_integration/apis/actions/fire.ts

@@ -64,9 +64,9 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) {
             message: 'Testing 123',
           },
         })
-        .expect(204)
+        .expect(200)
         .then((resp: any) => {
-          expect(resp.body).to.eql({});
+          expect(resp.body).to.be.an('object');
         });
       const indexedRecord = await retry.tryForTime(5000, async () => {
         const searchResult = await es.search({
@@ -146,9 +146,9 @@ export default function({ getService }: KibanaFunctionalTestDefaultProviders) {
             message: 'Testing 123',
           },
         })
-        .expect(204)
+        .expect(200)
         .then((resp: any) => {
-          expect(resp.body).to.eql({});
+          expect(resp.body).to.be.an('object');
         });
       const indexedRecord = await retry.tryForTime(5000, async () => {
         const searchResult = await es.search({