[APM] Don't log error if request was aborted (#105024)

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Jul 10, 2021 · 857dc9f · 857dc9f
1 parent c0fec48
commit 857dc9f
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 31 deletions.
diff --git a/x-pack/plugins/apm/server/routes/register_routes/index.test.ts b/x-pack/plugins/apm/server/routes/register_routes/index.test.ts
@@ -109,6 +109,11 @@ const initApi = (
         params: {},
         query: {},
         body: null,
+        events: {
+          aborted$: {
+            toPromise: () => new Promise(() => {}),
+          },
+        },
         ...request,
       },
       responseMock
@@ -202,7 +207,7 @@ describe('createApi', () => {
   describe('when validating', () => {
     describe('_inspect', () => {
       it('allows _inspect=true', async () => {
-        const handlerMock = jest.fn();
+        const handlerMock = jest.fn().mockResolvedValue({});
         const {
           simulateRequest,
           mocks: { response },
@@ -234,7 +239,7 @@ describe('createApi', () => {
       });
 
       it('rejects _inspect=1', async () => {
-        const handlerMock = jest.fn();
+        const handlerMock = jest.fn().mockResolvedValue({});
 
         const {
           simulateRequest,
@@ -267,7 +272,7 @@ describe('createApi', () => {
       });
 
       it('allows omitting _inspect', async () => {
-        const handlerMock = jest.fn();
+        const handlerMock = jest.fn().mockResolvedValue({});
 
         const {
           simulateRequest,
@@ -297,7 +302,11 @@ describe('createApi', () => {
         simulateRequest,
         mocks: { response },
       } = initApi([
-        { endpoint: 'GET /foo', options: { tags: [] }, handler: jest.fn() },
+        {
+          endpoint: 'GET /foo',
+          options: { tags: [] },
+          handler: jest.fn().mockResolvedValue({}),
+        },
       ]);
 
       await simulateRequest({
@@ -328,7 +337,7 @@ describe('createApi', () => {
     });
 
     it('validates path parameters', async () => {
-      const handlerMock = jest.fn();
+      const handlerMock = jest.fn().mockResolvedValue({});
       const {
         simulateRequest,
         mocks: { response },
@@ -402,7 +411,7 @@ describe('createApi', () => {
     });
 
     it('validates body parameters', async () => {
-      const handlerMock = jest.fn();
+      const handlerMock = jest.fn().mockResolvedValue({});
       const {
         simulateRequest,
         mocks: { response },
@@ -448,7 +457,7 @@ describe('createApi', () => {
     });
 
     it('validates query parameters', async () => {
-      const handlerMock = jest.fn();
+      const handlerMock = jest.fn().mockResolvedValue({});
       const {
         simulateRequest,
         mocks: { response },

diff --git a/x-pack/plugins/apm/server/routes/register_routes/index.ts b/x-pack/plugins/apm/server/routes/register_routes/index.ts
@@ -29,6 +29,13 @@ const inspectRt = t.exact(
   })
 );
 
+const CLIENT_CLOSED_REQUEST = {
+  statusCode: 499,
+  body: {
+    message: 'Client closed request',
+  },
+};
+
 export const inspectableEsQueriesMap = new WeakMap<
   KibanaRequest,
   InspectResponse
@@ -89,23 +96,40 @@ export function registerRoutes({
             runtimeType
           );
 
-          const data: Record<string, any> | undefined | null = (await handler({
-            request,
-            context,
-            config,
-            logger,
-            core,
-            plugins,
-            params: merge(
-              {
-                query: {
-                  _inspect: false,
+          const { aborted, data } = await Promise.race([
+            handler({
+              request,
+              context,
+              config,
+              logger,
+              core,
+              plugins,
+              params: merge(
+                {
+                  query: {
+                    _inspect: false,
+                  },
                 },
-              },
-              validatedParams
-            ),
-            ruleDataClient,
-          })) as any;
+                validatedParams
+              ),
+              ruleDataClient,
+            }).then((value) => {
+              return {
+                aborted: false,
+                data: value as Record<string, any> | undefined | null,
+              };
+            }),
+            request.events.aborted$.toPromise().then(() => {
+              return {
+                aborted: true,
+                data: undefined,
+              };
+            }),
+          ]);
+
+          if (aborted) {
+            return response.custom(CLIENT_CLOSED_REQUEST);
+          }
 
           if (Array.isArray(data)) {
             throw new Error('Return type cannot be an array');
@@ -118,9 +142,6 @@ export function registerRoutes({
               }
             : { ...data };
 
-          // cleanup
-          inspectableEsQueriesMap.delete(request);
-
           if (!options.disableTelemetry && telemetryUsageCounter) {
             telemetryUsageCounter.incrementCounter({
               counterName: `${method.toUpperCase()} ${pathname}`,
@@ -131,6 +152,7 @@ export function registerRoutes({
           return response.ok({ body });
         } catch (error) {
           logger.error(error);
+
           if (!options.disableTelemetry && telemetryUsageCounter) {
             telemetryUsageCounter.incrementCounter({
               counterName: `${method.toUpperCase()} ${pathname}`,
@@ -147,16 +169,18 @@ export function registerRoutes({
             },
           };
 
-          if (Boom.isBoom(error)) {
-            opts.statusCode = error.output.statusCode;
+          if (error instanceof RequestAbortedError) {
+            return response.custom(merge(opts, CLIENT_CLOSED_REQUEST));
           }
 
-          if (error instanceof RequestAbortedError) {
-            opts.statusCode = 499;
-            opts.body.message = 'Client closed request';
+          if (Boom.isBoom(error)) {
+            opts.statusCode = error.output.statusCode;
           }
 
           return response.custom(opts);
+        } finally {
+          // cleanup
+          inspectableEsQueriesMap.delete(request);
         }
       }
     );