gcp promises: allSettled instead of all

src/plugins/kibana_usage_collection/server/collectors/cloud/detector/azure.test.ts

@@ -62,20 +62,22 @@ describe('AzureCloudService', () => {
       const someError = new Error('expected: request failed');
       fetchMock.mockRejectedValue(someError);
 
-      expect(() => azureCloudService['_checkIfService']()).rejects.toThrowError(someError.message);
+      await expect(() => azureCloudService['_checkIfService']()).rejects.toThrowError(
+        someError.message
+      );
     });
 
     it('handles not running on Azure with 404 response by throwing error', async () => {
       fetchMock.mockResolvedValue({ status: 404 });
 
-      expect(() =>
+      await expect(() =>
         azureCloudService['_checkIfService']()
       ).rejects.toThrowErrorMatchingInlineSnapshot(`"Azure request failed"`);
     });
 
     it('handles not running on Azure with unexpected response by throwing error', async () => {
       fetchMock.mockResolvedValue({ ok: false });
-      expect(() =>
+      await expect(() =>
         azureCloudService['_checkIfService']()
       ).rejects.toThrowErrorMatchingInlineSnapshot(`"Azure request failed"`);
     });

src/plugins/kibana_usage_collection/server/collectors/cloud/detector/cloud_service.test.ts

@@ -30,9 +30,9 @@ describe('CloudService', () => {
 
   describe('_checkIfService', () => {
     it('throws an exception unless overridden', async () => {
-      expect(async () => {
-        await service._checkIfService(undefined);
-      }).rejects.toThrowErrorMatchingInlineSnapshot(`"not implemented"`);
+      await expect(() =>
+        service._checkIfService(undefined)
+      ).rejects.toThrowErrorMatchingInlineSnapshot(`"not implemented"`);
     });
   });
 
@@ -88,7 +88,7 @@ describe('CloudService', () => {
   describe('_parseResponse', () => {
     const body = { some: { body: {} } };
 
-    it('throws error upon failure to parse body as object', async () => {
+    it('throws error upon failure to parse body as object', () => {
       expect(() => service._parseResponse()).toThrowErrorMatchingInlineSnapshot(
         `"Unable to handle body"`
       );

src/plugins/kibana_usage_collection/server/collectors/cloud/detector/gcp.test.ts

@@ -79,8 +79,8 @@ describe('GCP', () => {
         text: () => undefined,
       });
 
-      expect(
-        async () => await gcpService['_checkIfService']()
+      await expect(() =>
+        gcpService['_checkIfService']()
       ).rejects.toThrowErrorMatchingInlineSnapshot(`"unrecognized responses"`);
     });
 
@@ -92,16 +92,18 @@ describe('GCP', () => {
         text: () => 'xyz',
       });
 
-      expect(
-        async () => await gcpService['_checkIfService']()
+      await expect(() =>
+        gcpService['_checkIfService']()
       ).rejects.toThrowErrorMatchingInlineSnapshot(`"GCP request failed"`);
     });
 
-    it('handles not running on GCP with error by rethrowing it', async () => {
+    it('handles not running on GCP', async () => {
       const someError = new Error('expected: request failed');
       fetchMock.mockRejectedValue(someError);
 
-      expect(async () => await gcpService['_checkIfService']()).rejects.toThrowError(someError);
+      await expect(() =>
+        gcpService['_checkIfService']()
+      ).rejects.toThrowErrorMatchingInlineSnapshot(`"GCP request failed"`);
     });
 
     it('handles not running on GCP with 404 response by throwing error', async () => {
@@ -112,10 +114,47 @@ describe('GCP', () => {
         text: () => 'This is some random error text',
       });
 
-      expect(
-        async () => await gcpService['_checkIfService']()
+      await expect(() =>
+        gcpService['_checkIfService']()
       ).rejects.toThrowErrorMatchingInlineSnapshot(`"GCP request failed"`);
     });
+
+    it('handles GCP response even if some requests fail', async () => {
+      fetchMock
+        .mockResolvedValueOnce({
+          status: 200,
+          ok: true,
+          headers,
+          text: () => 'some_id',
+        })
+        .mockRejectedValueOnce({
+          status: 500,
+          ok: false,
+          headers,
+          text: () => 'This is some random error text',
+        })
+        .mockResolvedValueOnce({
+          status: 404,
+          ok: false,
+          headers,
+          text: () => 'URI Not found',
+        });
+      const response = await gcpService['_checkIfService']();
+
+      expect(fetchMock).toBeCalledTimes(3);
+
+      expect(response.isConfirmed()).toEqual(true);
+      expect(response.toJSON()).toMatchInlineSnapshot(`
+        Object {
+          "id": "some_id",
+          "metadata": undefined,
+          "name": "gcp",
+          "region": undefined,
+          "vm_type": undefined,
+          "zone": undefined,
+        }
+      `);
+    });
   });
 
   describe('extractValue', () => {
@@ -177,9 +216,7 @@ describe('GCP', () => {
     });
 
     it('ignores unexpected response body', () => {
-      // @ts-expect-error
       expect(() => gcpService['combineResponses']()).toThrow();
-      // @ts-expect-error
       expect(() => gcpService['combineResponses'](undefined, undefined, undefined)).toThrow();
       // @ts-expect-error
       expect(() => gcpService['combineResponses'](null, null, null)).toThrow();

src/plugins/kibana_usage_collection/server/collectors/cloud/detector/gcp.ts

@@ -6,7 +6,7 @@
  * Side Public License, v 1.
  */
 
-import fetch from 'node-fetch';
+import fetch, { Response } from 'node-fetch';
 import { CloudService } from './cloud_service';
 import { CloudServiceResponse } from './cloud_response';
 
@@ -28,7 +28,7 @@ export class GCPCloudService extends CloudService {
     // we need to call GCP individually for each field we want metadata for
     const fields = ['id', 'machine-type', 'zone'];
 
-    const responses = await Promise.all(
+    const settledResponses = await Promise.allSettled(
       fields.map(async (field) => {
         return await fetch(`${SERVICE_ENDPOINT}/${field}`, {
           method: 'GET',
@@ -40,28 +40,38 @@ export class GCPCloudService extends CloudService {
       })
     );
 
+    const hasValidResponses = settledResponses.some(this.isValidResponse);
+
+    if (!hasValidResponses) {
+      throw new Error('GCP request failed');
+    }
+
     // Note: there is no fallback option for GCP;
     // responses are arrays containing [fullResponse, body];
     // because GCP returns plaintext, we have no way of validating
     // without using the response code.
     const [id, machineType, zone] = await Promise.all(
-      responses.map(async (response) => {
-        if (
-          !response.ok ||
-          response.status === 404 ||
-          response.headers.get('metadata-flavor') !== 'Google'
-        ) {
-          throw new Error('GCP request failed');
+      settledResponses.map(async (settledResponse) => {
+        if (this.isValidResponse(settledResponse)) {
+          // GCP does _not_ return JSON
+          return await settledResponse.value.text();
         }
-
-        // GCP does _not_ return JSON
-        return await response.text();
       })
     );
 
     return this.combineResponses(id, machineType, zone);
   };
 
+  private isValidResponse = (
+    settledResponse: PromiseSettledResult<Response>
+  ): settledResponse is PromiseFulfilledResult<Response> => {
+    if (settledResponse.status === 'rejected') {
+      return false;
+    }
+    const { value } = settledResponse;
+    return value.ok && value.status !== 404 && value.headers.get('metadata-flavor') === 'Google';
+  };
+
   /**
    * Parse the GCP responses, if possible.
    *
@@ -71,17 +81,11 @@ export class GCPCloudService extends CloudService {
    * machineType: 'projects/441331612345/machineTypes/f1-micro'
    * zone: 'projects/441331612345/zones/us-east4-c'
    */
-  private combineResponses = (id: string, machineType: string, zone: string) => {
+  private combineResponses = (id?: string, machineType?: string, zone?: string) => {
     const vmId = typeof id === 'string' ? id.trim() : undefined;
     const vmType = this.extractValue('machineTypes/', machineType);
     const vmZone = this.extractValue('zones/', zone);
-
-    let region;
-
-    if (vmZone) {
-      // converts 'us-east4-c' into 'us-east4'
-      region = vmZone.substring(0, vmZone.lastIndexOf('-'));
-    }
+    const region = vmZone ? vmZone.substring(0, vmZone.lastIndexOf('-')) : undefined;
 
     // ensure we actually have some data
     if (vmId || vmType || region || vmZone) {
@@ -98,15 +102,15 @@ export class GCPCloudService extends CloudService {
    * For example, this turns something like
    * 'projects/441331612345/machineTypes/f1-micro' into 'f1-micro'.
    */
-  private extractValue = (fieldPrefix: string, value: string) => {
-    if (typeof value === 'string') {
-      const index = value.lastIndexOf(fieldPrefix);
-
-      if (index !== -1) {
-        return value.substring(index + fieldPrefix.length).trim();
-      }
+  private extractValue = (fieldPrefix: string, value?: string) => {
+    if (typeof value !== 'string') {
+      return;
     }
 
-    return undefined;
+    const index = value.lastIndexOf(fieldPrefix);
+
+    if (index !== -1) {
+      return value.substring(index + fieldPrefix.length).trim();
+    }
   };
 }