fix: handle components with empty string as their type

package.json

@@ -36,8 +36,7 @@
     "jszip": "^3.10.1",
     "mime": "2.6.0",
     "minimatch": "^5.1.6",
-    "proxy-agent": "^6.4.0",
-    "ts-retry-promise": "^0.7.1"
+    "proxy-agent": "^6.4.0"
   },
   "devDependencies": {
     "@jsforce/jsforce-node": "^3.1.0",

src/resolve/connectionResolver.ts

@@ -5,9 +5,7 @@
  * For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
  */
 
-import { retry, NotRetryableError, RetryError } from 'ts-retry-promise';
-import { PollingClient, StatusResult, Connection, Logger, Messages, Lifecycle, SfError } from '@salesforce/core';
-import { Duration, ensureArray } from '@salesforce/kit';
+import { Connection, Logger, Messages, Lifecycle, SfError } from '@salesforce/core';
 import { ensurePlainObject, ensureString, isPlainObject } from '@salesforce/ts-types';
 import { RegistryAccess } from '../registry/registryAccess';
 import { MetadataType } from '../registry/types';
@@ -57,21 +55,21 @@ export class ConnectionResolver {
     const lifecycle = Lifecycle.getInstance();
 
     const componentFromDescribe = (
-      await Promise.all(this.mdTypeNames.map((type) => this.listMembers({ type })))
+      await Promise.all(this.mdTypeNames.map((type) => listMembers(this.registry)(this.connection)({ type })))
     ).flat();
 
     for (const component of componentFromDescribe) {
       let componentType: MetadataType;
-      if (typeof component.type === 'string' && component.type.length) {
+      if (isNonEmptyString(component.type)) {
         componentType = this.registry.getTypeByName(component.type);
-      } else if (typeof component.fileName === 'string' && component.fileName.length) {
+      } else if (isNonEmptyString(component.fileName)) {
         // fix { type: { "$": { "xsi:nil": "true" } } }
         componentType = ensurePlainObject(
           this.registry.getTypeBySuffix(extName(component.fileName)),
           `No type found for ${component.fileName} when matching by suffix.  Check the file extension.`
         );
         component.type = componentType.name;
-      } else if (component.type === undefined && component.fileName === undefined) {
+      } else if (!isNonEmptyString(component.type) && !isNonEmptyString(component.fileName)) {
         // has no type and has no filename!  Warn and skip that component.
         // eslint-disable-next-line no-await-in-loop
         await Promise.all([
@@ -91,11 +89,10 @@ export class ConnectionResolver {
 
       Aggregator.push(component);
       componentTypes.add(componentType);
-      const folderContentType = componentType.folderContentType;
-      if (folderContentType) {
+      if (componentType.folderContentType) {
         childrenPromises.push(
-          this.listMembers({
-            type: this.registry.getTypeByName(folderContentType).name,
+          listMembers(this.registry)(this.connection)({
+            type: this.registry.getTypeByName(componentType.folderContentType).name,
             folder: component.fullName,
           })
         );
@@ -106,7 +103,7 @@ export class ConnectionResolver {
       const childTypes = componentType.children?.types;
       if (childTypes) {
         Object.values(childTypes).map((childType) => {
-          childrenPromises.push(this.listMembers({ type: childType.name }));
+          childrenPromises.push(listMembers(this.registry)(this.connection)({ type: childType.name }));
         });
       }
     }
@@ -125,93 +122,60 @@ export class ConnectionResolver {
       apiVersion: this.connection.getApiVersion(),
     };
   }
+}
 
-  private async listMembers(query: ListMetadataQuery): Promise<RelevantFileProperties[]> {
-    let members: RelevantFileProperties[] = [];
-
-    const pollingOptions: PollingClient.Options = {
-      frequency: Duration.milliseconds(1000),
-      timeout: Duration.minutes(3),
-      poll: async (): Promise<StatusResult> => {
-        const res = ensureArray(await this.connection.metadata.list(query));
-        return { completed: true, payload: res };
-      },
-    };
-
-    const pollingClient = await PollingClient.create(pollingOptions);
-
-    try {
-      members = await pollingClient.subscribe();
-    } catch (error) {
-      // throw error if PollingClient timed out.
-      if (error instanceof NotRetryableError) {
-        throw NotRetryableError;
-      }
-      this.logger.debug((error as Error).message);
-      members = [];
-    }
-
-    // if the Metadata Type doesn't return a correct fileName then help it out
-    for (const m of members) {
-      if (typeof m.fileName == 'object') {
-        const t = this.registry.getTypeByName(query.type);
-        m.fileName = `${t.directoryName}/${m.fullName}.${t.suffix}`;
-      }
-    }
+const listMembers =
+  (registry: RegistryAccess) =>
+  (connection: Connection) =>
+  async (query: ListMetadataQuery): Promise<RelevantFileProperties[]> => {
+    const mdType = registry.getTypeByName(query.type);
 
     // Workaround because metadata.list({ type: 'StandardValueSet' }) returns []
-    if (query.type === this.registry.getRegistry().types.standardvalueset.name && members.length === 0) {
+    if (mdType.name === registry.getRegistry().types.standardvalueset.name) {
+      const members: RelevantFileProperties[] = [];
+
       const standardValueSetPromises = standardValueSet.fullnames.map(async (standardValueSetFullName) => {
         try {
-          // The 'singleRecordQuery' method was having connection errors, using `retry` resolves this
-          // Note that this type of connection retry logic may someday be added to jsforce v2
-          // Once that happens this logic could be reverted
-          const standardValueSetRecord: StdValueSetRecord = await retry(async () => {
-            try {
-              return await this.connection.singleRecordQuery(
-                `SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = '${standardValueSetFullName}'`,
-                { tooling: true }
-              );
-            } catch (err) {
-              // We exit the retry loop with `NotRetryableError` if we get an (expected) unsupported metadata type error
-              const error = err as Error;
-              if (error.message.includes('either inaccessible or not supported in Metadata API')) {
-                this.logger.debug('Expected error:', error.message);
-                throw new NotRetryableError(error.message);
-              }
-
-              // Otherwise throw the err so we can retry again
-              throw err;
-            }
-          });
+          const standardValueSetRecord: StdValueSetRecord = await connection.singleRecordQuery(
+            `SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = '${standardValueSetFullName}'`,
+            { tooling: true }
+          );
 
           return (
             standardValueSetRecord.Metadata.standardValue.length && {
               fullName: standardValueSetRecord.MasterLabel,
-              fileName: `${this.registry.getRegistry().types.standardvalueset.directoryName}/${
-                standardValueSetRecord.MasterLabel
-              }.${this.registry.getRegistry().types.standardvalueset.suffix}`,
-              type: this.registry.getRegistry().types.standardvalueset.name,
+              fileName: `${mdType.directoryName}/${standardValueSetRecord.MasterLabel}.${mdType.suffix}`,
+              type: mdType.name,
             }
           );
         } catch (err) {
-          // error.message here will be overwritten by 'ts-retry-promise'
-          // Example error.message from the library: "All retries failed" or "Met not retryable error"
-          // 'ts-retry-promise' exposes the actual error on `error.lastError`
-          const error = err as RetryError;
-
-          if (error.lastError?.message) {
-            this.logger.debug(error.lastError.message);
-          }
+          const logger = Logger.childFromRoot('ConnectionResolver.listMembers');
+          logger.debug(err);
         }
       });
       for await (const standardValueSetResult of standardValueSetPromises) {
         if (standardValueSetResult) {
           members.push(standardValueSetResult);
         }
       }
+      return members;
     }
 
-    return members;
-  }
-}
+    try {
+      return (await connection.metadata.list(query)).map(inferFilenamesFromType(mdType));
+    } catch (error) {
+      const logger = Logger.childFromRoot('ConnectionResolver.listMembers');
+      logger.debug((error as Error).message);
+      return [];
+    }
+  };
+
+/* if the Metadata Type doesn't return a correct fileName then help it out */
+const inferFilenamesFromType =
+  (metadataType: MetadataType) =>
+  (member: RelevantFileProperties): RelevantFileProperties =>
+    typeof member.fileName === 'object'
+      ? { ...member, fileName: `${metadataType.directoryName}/${member.fullName}.${metadataType.suffix}` }
+      : member;
+
+const isNonEmptyString = (value: string | undefined): boolean => typeof value === 'string' && value.length > 0;

test/resolve/connectionResolver.test.ts

@@ -7,7 +7,7 @@
 
 import { assert, expect, use } from 'chai';
 import { MockTestOrgData, TestContext } from '@salesforce/core/testSetup';
-import { Connection, Logger } from '@salesforce/core';
+import { Connection } from '@salesforce/core';
 import deepEqualInAnyOrder from 'deep-equal-in-any-order';
 import { ManageableState } from '../../src/client/types';
 import { ConnectionResolver } from '../../src/resolve';
@@ -291,51 +291,6 @@ describe('ConnectionResolver', () => {
       expect(result.components).to.deep.equal(expected);
     });
 
-    it('should retry (ten times) if unexpected error occurs', async () => {
-      const loggerStub = $$.SANDBOX.stub(Logger.prototype, 'debug');
-
-      $$.SANDBOX.stub(connection.metadata, 'list');
-
-      const query = "SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = 'AccountOwnership'";
-
-      const mockToolingQuery = $$.SANDBOX.stub(connection, 'singleRecordQuery');
-      mockToolingQuery.withArgs(query).rejects(new Error('Something happened. Oh no.'));
-
-      const resolver = new ConnectionResolver(connection);
-      const result = await resolver.resolve();
-      const expected: MetadataComponent[] = [];
-
-      // filter over queries and find ones called with `query`
-      const retries = mockToolingQuery.args.filter((call) => call[0] === query);
-
-      expect(retries.length).to.equal(11); // first call plus 10 retries
-      expect(loggerStub.calledOnce).to.be.true;
-      expect(loggerStub.args[0][0]).to.equal('Something happened. Oh no.');
-      expect(result.components).to.deep.equal(expected);
-    });
-
-    it('should not retry query if expected unsupported metadata error is encountered', async () => {
-      const loggerStub = $$.SANDBOX.stub(Logger.prototype, 'debug');
-
-      $$.SANDBOX.stub(connection.metadata, 'list');
-
-      const errorMessage = 'WorkTypeGroupAddInfo is either inaccessible or not supported in Metadata API';
-
-      const mockToolingQuery = $$.SANDBOX.stub(connection, 'singleRecordQuery');
-      mockToolingQuery
-        .withArgs("SELECT Id, MasterLabel, Metadata FROM StandardValueSet WHERE MasterLabel = 'WorkTypeGroupAddInfo'")
-        .rejects(new Error(errorMessage));
-
-      const resolver = new ConnectionResolver(connection);
-      const result = await resolver.resolve();
-      const expected: MetadataComponent[] = [];
-
-      expect(loggerStub.calledOnce).to.be.true;
-      expect(loggerStub.args[0][0]).to.equal('Expected error:');
-      expect(loggerStub.args[0][1]).to.equal(errorMessage);
-      expect(result.components).to.deep.equal(expected);
-    });
-
     it('should resolve no managed components', async () => {
       const metadataQueryStub = $$.SANDBOX.stub(connection.metadata, 'list');
 

yarn.lock

@@ -5017,16 +5017,7 @@ srcset@^5.0.0:
   resolved "https://registry.yarnpkg.com/srcset/-/srcset-5.0.0.tgz#9df6c3961b5b44a02532ce6ae4544832609e2e3f"
   integrity sha512-SqEZaAEhe0A6ETEa9O1IhSPC7MdvehZtCnTR0AftXk3QhY2UNgb+NApFOUPZILXk/YTDfFxMTNJOBpzrJsEdIA==
 
-"string-width-cjs@npm:string-width@^4.2.0":
-  version "4.2.3"
-  resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
-  integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
-  dependencies:
-    emoji-regex "^8.0.0"
-    is-fullwidth-code-point "^3.0.0"
-    strip-ansi "^6.0.1"
-
-string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
+"string-width-cjs@npm:string-width@^4.2.0", string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
   version "4.2.3"
   resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
   integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
@@ -5085,14 +5076,7 @@ string_decoder@~1.1.1:
   dependencies:
     safe-buffer "~5.1.0"
 
-"strip-ansi-cjs@npm:strip-ansi@^6.0.1":
-  version "6.0.1"
-  resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
-  integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
-  dependencies:
-    ansi-regex "^5.0.1"
-
-[email protected], strip-ansi@^6.0.0, strip-ansi@^6.0.1:
+"strip-ansi-cjs@npm:strip-ansi@^6.0.1", [email protected], strip-ansi@^6.0.0, strip-ansi@^6.0.1:
   version "6.0.1"
   resolved "https://registry.yarnpkg.com/strip-ansi/-/strip-ansi-6.0.1.tgz#9e26c63d30f53443e9489495b2105d37b67a85d9"
   integrity sha512-Y38VPSHcqkFrCpFnQ9vuSXmquuv5oXOKpGeT6aGrr3o3Gc9AlVa6JBfUSOCnbxGGZF+/0ooI7KrPuUSztUdU5A==
@@ -5591,7 +5575,7 @@ [email protected]:
   resolved "https://registry.yarnpkg.com/workerpool/-/workerpool-6.2.1.tgz#46fc150c17d826b86a008e5a4508656777e9c343"
   integrity sha512-ILEIE97kDZvF9Wb9f6h5aXK4swSlKGUcOEGiIYb2OOu/IrDU9iwj0fD//SsA6E5ibwJxpEvhullJY4Sl4GcpAw==
 
-"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0":
+"wrap-ansi-cjs@npm:wrap-ansi@^7.0.0", wrap-ansi@^7.0.0:
   version "7.0.0"
   resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
   integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
@@ -5609,15 +5593,6 @@ wrap-ansi@^6.2.0:
     string-width "^4.1.0"
     strip-ansi "^6.0.0"
 
-wrap-ansi@^7.0.0:
-  version "7.0.0"
-  resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-7.0.0.tgz#67e145cff510a6a6984bdf1152911d69d2eb9e43"
-  integrity sha512-YVGIj2kamLSTxw6NsZjoBxfSwsn0ycdesmc4p+Q21c5zPuZ1pl+NfxVdxPtdHvmNVOQ6XSYG4AUtyt/Fi7D16Q==
-  dependencies:
-    ansi-styles "^4.0.0"
-    string-width "^4.1.0"
-    strip-ansi "^6.0.0"
-
 wrap-ansi@^8.1.0:
   version "8.1.0"
   resolved "https://registry.yarnpkg.com/wrap-ansi/-/wrap-ansi-8.1.0.tgz#56dc22368ee570face1b49819975d9b9a5ead214"