From 30b64b4a6d22d5fb65b66df264393a0e23625114 Mon Sep 17 00:00:00 2001
From: George Fu <kuhe@users.noreply.github.com>
Date: Thu, 23 May 2024 20:13:42 +0000
Subject: [PATCH 1/6] chore: add logging for CredentialsProviderError

---
 .changeset/two-weeks-provide.md                   |  6 ++++++
 packages/node-config-provider/src/fromEnv.ts      |  4 +++-
 .../src/fromSharedConfigFiles.ts                  |  5 +++--
 .../node-config-provider/src/getSelectorName.ts   | 15 +++++++++++++++
 .../src/CredentialsProviderError.spec.ts          | 14 ++++++++++++++
 .../src/CredentialsProviderError.ts               | 11 ++++++++---
 6 files changed, 49 insertions(+), 6 deletions(-)
 create mode 100644 .changeset/two-weeks-provide.md
 create mode 100644 packages/node-config-provider/src/getSelectorName.ts

diff --git a/.changeset/two-weeks-provide.md b/.changeset/two-weeks-provide.md
new file mode 100644
index 00000000000..5854163089b
--- /dev/null
+++ b/.changeset/two-weeks-provide.md
@@ -0,0 +1,6 @@
+---
+"@smithy/node-config-provider": patch
+"@smithy/property-provider": patch
+---
+
+add logging capability for CredentialsProviderError
diff --git a/packages/node-config-provider/src/fromEnv.ts b/packages/node-config-provider/src/fromEnv.ts
index 7d28654b30b..6e4b4e9faaf 100644
--- a/packages/node-config-provider/src/fromEnv.ts
+++ b/packages/node-config-provider/src/fromEnv.ts
@@ -1,6 +1,8 @@
 import { CredentialsProviderError } from "@smithy/property-provider";
 import { Provider } from "@smithy/types";
 
+import { getSelectorName } from "./getSelectorName";
+
 // Using Record<string, string | undefined> instead of NodeJS.ProcessEnv, in order to not get type errors in non node environments
 export type GetterFromEnv<T> = (env: Record<string, string | undefined>) => T | undefined;
 
@@ -19,7 +21,7 @@ export const fromEnv =
       return config as T;
     } catch (e) {
       throw new CredentialsProviderError(
-        e.message || `Cannot load config from environment variables with getter: ${envVarSelector}`
+        e.message || `Not found in ENV: ${getSelectorName(envVarSelector.toString())}`
       );
     }
   };
diff --git a/packages/node-config-provider/src/fromSharedConfigFiles.ts b/packages/node-config-provider/src/fromSharedConfigFiles.ts
index 66a5e0ab511..cb28d71cffd 100644
--- a/packages/node-config-provider/src/fromSharedConfigFiles.ts
+++ b/packages/node-config-provider/src/fromSharedConfigFiles.ts
@@ -2,6 +2,8 @@ import { CredentialsProviderError } from "@smithy/property-provider";
 import { getProfileName, loadSharedConfigFiles, SourceProfileInit } from "@smithy/shared-ini-file-loader";
 import { ParsedIniData, Profile, Provider } from "@smithy/types";
 
+import { getSelectorName } from "./getSelectorName";
+
 export interface SharedConfigInit extends SourceProfileInit {
   /**
    * The preferred shared ini file to load the config. "config" option refers to
@@ -41,8 +43,7 @@ export const fromSharedConfigFiles =
       return configValue;
     } catch (e) {
       throw new CredentialsProviderError(
-        e.message ||
-          `Cannot load config for profile ${profile} in SDK configuration files with getter: ${configSelector}`
+        e.message || `Not found in config files w/ profile [${profile}]: ${getSelectorName(configSelector.toString())}`
       );
     }
   };
diff --git a/packages/node-config-provider/src/getSelectorName.ts b/packages/node-config-provider/src/getSelectorName.ts
new file mode 100644
index 00000000000..58a6fa68422
--- /dev/null
+++ b/packages/node-config-provider/src/getSelectorName.ts
@@ -0,0 +1,15 @@
+/**
+ * Attempts to extract the name of the variable that the functional selector is looking for.
+ * Improves readability over the raw Function.toString() value.
+ *
+ * @param functionString - function's string representation.
+ *
+ * @returns constant value used within the function.
+ */
+export function getSelectorName(functionString: string): string {
+  const constants = new Set(Array.from(functionString.match(/([A-Z_]){3,}/g) ?? []));
+  constants.delete("CONFIG");
+  constants.delete("CONFIG_PREFIX_SEPARATOR");
+  constants.delete("ENV");
+  return [...constants].join(", ");
+}
diff --git a/packages/property-provider/src/CredentialsProviderError.spec.ts b/packages/property-provider/src/CredentialsProviderError.spec.ts
index db951afac61..7b6f9caa550 100644
--- a/packages/property-provider/src/CredentialsProviderError.spec.ts
+++ b/packages/property-provider/src/CredentialsProviderError.spec.ts
@@ -2,10 +2,24 @@ import { CredentialsProviderError } from "./CredentialsProviderError";
 import { ProviderError } from "./ProviderError";
 
 describe(CredentialsProviderError.name, () => {
+  afterAll(() => {
+    CredentialsProviderError.logLimit = 100;
+  });
+
   it("should be named as CredentialsProviderError", () => {
     expect(new CredentialsProviderError("PANIC").name).toBe("CredentialsProviderError");
   });
 
+  it("should log up to its logLimit", () => {
+    CredentialsProviderError.logLimit = 5;
+
+    Array.from({ length: CredentialsProviderError.logLimit + 2 }).forEach(() => {
+      new CredentialsProviderError("PANIC");
+    });
+
+    expect(CredentialsProviderError.log.length).toEqual(CredentialsProviderError.logLimit);
+  });
+
   describe.each([Error, ProviderError, CredentialsProviderError])("should be instanceof %p", (classConstructor) => {
     it("when created using constructor", () => {
       expect(new CredentialsProviderError("PANIC")).toBeInstanceOf(classConstructor);
diff --git a/packages/property-provider/src/CredentialsProviderError.ts b/packages/property-provider/src/CredentialsProviderError.ts
index c5a62f12ef4..50c1e8b8b14 100644
--- a/packages/property-provider/src/CredentialsProviderError.ts
+++ b/packages/property-provider/src/CredentialsProviderError.ts
@@ -12,13 +12,18 @@ import { ProviderError } from "./ProviderError";
  * ensures the chain will stop if an entirely unexpected error is encountered.
  */
 export class CredentialsProviderError extends ProviderError {
-  name = "CredentialsProviderError";
-  constructor(
+  public static log: string[] = [];
+  public static logLimit = 100;
+  public name = "CredentialsProviderError";
+  public constructor(
     message: string,
     public readonly tryNextLink: boolean = true
   ) {
     super(message, tryNextLink);
-    // Remove once we stop targetting ES5.
     Object.setPrototypeOf(this, CredentialsProviderError.prototype);
+    CredentialsProviderError.log.push(`${new Date().toISOString()} ${tryNextLink ? "->" : "(!)"} ${message}`);
+    while (CredentialsProviderError.log.length > CredentialsProviderError.logLimit) {
+      CredentialsProviderError.log.shift();
+    }
   }
 }

From 94b84ec6577d46854e960019e00479bb0527c65c Mon Sep 17 00:00:00 2001
From: George Fu <kuhe@users.noreply.github.com>
Date: Fri, 24 May 2024 15:11:52 +0000
Subject: [PATCH 2/6] use options object

---
 .changeset/late-glasses-jam.md                |  8 ++++
 .changeset/two-weeks-provide.md               |  6 ---
 .../src/fromContainerMetadata.ts              | 31 +++++++-----
 .../src/fromInstanceMetadata.ts               | 11 +++--
 packages/node-config-provider/src/fromEnv.ts  |  7 +--
 .../src/fromSharedConfigFiles.ts              |  3 +-
 .../src/CredentialsProviderError.spec.ts      | 22 ++++-----
 .../src/CredentialsProviderError.ts           | 47 +++++++++++++++----
 .../src/loadSharedConfigFiles.ts              |  7 ++-
 9 files changed, 93 insertions(+), 49 deletions(-)
 create mode 100644 .changeset/late-glasses-jam.md
 delete mode 100644 .changeset/two-weeks-provide.md

diff --git a/.changeset/late-glasses-jam.md b/.changeset/late-glasses-jam.md
new file mode 100644
index 00000000000..1f45828f3f5
--- /dev/null
+++ b/.changeset/late-glasses-jam.md
@@ -0,0 +1,8 @@
+---
+"@smithy/credential-provider-imds": minor
+"@smithy/shared-ini-file-loader": minor
+"@smithy/node-config-provider": minor
+"@smithy/property-provider": minor
+---
+
+new logging-compatible signature for CredentialsProviderError
diff --git a/.changeset/two-weeks-provide.md b/.changeset/two-weeks-provide.md
deleted file mode 100644
index 5854163089b..00000000000
--- a/.changeset/two-weeks-provide.md
+++ /dev/null
@@ -1,6 +0,0 @@
----
-"@smithy/node-config-provider": patch
-"@smithy/property-provider": patch
----
-
-add logging capability for CredentialsProviderError
diff --git a/packages/credential-provider-imds/src/fromContainerMetadata.ts b/packages/credential-provider-imds/src/fromContainerMetadata.ts
index 0336931c361..2cfdff55568 100644
--- a/packages/credential-provider-imds/src/fromContainerMetadata.ts
+++ b/packages/credential-provider-imds/src/fromContainerMetadata.ts
@@ -1,5 +1,5 @@
 import { CredentialsProviderError } from "@smithy/property-provider";
-import { AwsCredentialIdentityProvider } from "@smithy/types";
+import { AwsCredentialIdentityProvider, Logger } from "@smithy/types";
 import { RequestOptions } from "http";
 import { parse } from "url";
 
@@ -31,10 +31,12 @@ export const fromContainerMetadata = (init: RemoteProviderInit = {}): AwsCredent
   const { timeout, maxRetries } = providerConfigFromInit(init);
   return () =>
     retry(async () => {
-      const requestOptions = await getCmdsUri();
+      const requestOptions = await getCmdsUri({ logger: init.logger });
       const credsResponse = JSON.parse(await requestFromEcsImds(timeout, requestOptions));
       if (!isImdsCredentials(credsResponse)) {
-        throw new CredentialsProviderError("Invalid response received from instance metadata service.");
+        throw new CredentialsProviderError("Invalid response received from instance metadata service.", {
+          logger: init.logger,
+        });
       }
       return fromImdsCredentials(credsResponse);
     }, maxRetries);
@@ -65,7 +67,7 @@ const GREENGRASS_PROTOCOLS = {
   "https:": true,
 };
 
-const getCmdsUri = async (): Promise<RequestOptions> => {
+const getCmdsUri = async ({ logger }: { logger?: Logger }): Promise<RequestOptions> => {
   if (process.env[ENV_CMDS_RELATIVE_URI]) {
     return {
       hostname: CMDS_IP,
@@ -76,17 +78,17 @@ const getCmdsUri = async (): Promise<RequestOptions> => {
   if (process.env[ENV_CMDS_FULL_URI]) {
     const parsed = parse(process.env[ENV_CMDS_FULL_URI]!);
     if (!parsed.hostname || !(parsed.hostname in GREENGRASS_HOSTS)) {
-      throw new CredentialsProviderError(
-        `${parsed.hostname} is not a valid container metadata service hostname`,
-        false
-      );
+      throw new CredentialsProviderError(`${parsed.hostname} is not a valid container metadata service hostname`, {
+        tryNextLink: false,
+        logger,
+      });
     }
 
     if (!parsed.protocol || !(parsed.protocol in GREENGRASS_PROTOCOLS)) {
-      throw new CredentialsProviderError(
-        `${parsed.protocol} is not a valid container metadata service protocol`,
-        false
-      );
+      throw new CredentialsProviderError(`${parsed.protocol} is not a valid container metadata service protocol`, {
+        tryNextLink: false,
+        logger,
+      });
     }
 
     return {
@@ -99,6 +101,9 @@ const getCmdsUri = async (): Promise<RequestOptions> => {
     "The container metadata credential provider cannot be used unless" +
       ` the ${ENV_CMDS_RELATIVE_URI} or ${ENV_CMDS_FULL_URI} environment` +
       " variable is set",
-    false
+    {
+      tryNextLink: false,
+      logger,
+    }
   );
 };
diff --git a/packages/credential-provider-imds/src/fromInstanceMetadata.ts b/packages/credential-provider-imds/src/fromInstanceMetadata.ts
index 875d238b542..0d9caf8dee3 100644
--- a/packages/credential-provider-imds/src/fromInstanceMetadata.ts
+++ b/packages/credential-provider-imds/src/fromInstanceMetadata.ts
@@ -47,7 +47,8 @@ const getInstanceImdsProvider = (init: RemoteProviderInit) => {
             fallbackBlockedFromProcessEnv = !!envValue && envValue !== "false";
             if (envValue === undefined) {
               throw new CredentialsProviderError(
-                `${AWS_EC2_METADATA_V1_DISABLED} not set in env, checking config file next.`
+                `${AWS_EC2_METADATA_V1_DISABLED} not set in env, checking config file next.`,
+                { logger: init.logger }
               );
             }
             return fallbackBlockedFromProcessEnv;
@@ -98,7 +99,7 @@ const getInstanceImdsProvider = (init: RemoteProviderInit) => {
     return retry(async () => {
       let creds: AwsCredentialIdentity;
       try {
-        creds = await getCredentialsFromProfile(imdsProfile, options);
+        creds = await getCredentialsFromProfile(imdsProfile, options, init);
       } catch (err) {
         if (err.statusCode === 401) {
           disableFetchToken = false;
@@ -152,7 +153,7 @@ const getMetadataToken = async (options: RequestOptions) =>
 
 const getProfile = async (options: RequestOptions) => (await httpRequest({ ...options, path: IMDS_PATH })).toString();
 
-const getCredentialsFromProfile = async (profile: string, options: RequestOptions) => {
+const getCredentialsFromProfile = async (profile: string, options: RequestOptions, init: RemoteProviderInit) => {
   const credsResponse = JSON.parse(
     (
       await httpRequest({
@@ -163,7 +164,9 @@ const getCredentialsFromProfile = async (profile: string, options: RequestOption
   );
 
   if (!isImdsCredentials(credsResponse)) {
-    throw new CredentialsProviderError("Invalid response received from instance metadata service.");
+    throw new CredentialsProviderError("Invalid response received from instance metadata service.", {
+      logger: init.logger,
+    });
   }
 
   return fromImdsCredentials(credsResponse);
diff --git a/packages/node-config-provider/src/fromEnv.ts b/packages/node-config-provider/src/fromEnv.ts
index 6e4b4e9faaf..33941b27fcc 100644
--- a/packages/node-config-provider/src/fromEnv.ts
+++ b/packages/node-config-provider/src/fromEnv.ts
@@ -1,5 +1,5 @@
 import { CredentialsProviderError } from "@smithy/property-provider";
-import { Provider } from "@smithy/types";
+import { Logger, Provider } from "@smithy/types";
 
 import { getSelectorName } from "./getSelectorName";
 
@@ -11,7 +11,7 @@ export type GetterFromEnv<T> = (env: Record<string, string | undefined>) => T |
  * environment variable.
  */
 export const fromEnv =
-  <T = string>(envVarSelector: GetterFromEnv<T>): Provider<T> =>
+  <T = string>(envVarSelector: GetterFromEnv<T>, logger?: Logger): Provider<T> =>
   async () => {
     try {
       const config = envVarSelector(process.env);
@@ -21,7 +21,8 @@ export const fromEnv =
       return config as T;
     } catch (e) {
       throw new CredentialsProviderError(
-        e.message || `Not found in ENV: ${getSelectorName(envVarSelector.toString())}`
+        e.message || `Not found in ENV: ${getSelectorName(envVarSelector.toString())}`,
+        { logger }
       );
     }
   };
diff --git a/packages/node-config-provider/src/fromSharedConfigFiles.ts b/packages/node-config-provider/src/fromSharedConfigFiles.ts
index cb28d71cffd..4947aaaa118 100644
--- a/packages/node-config-provider/src/fromSharedConfigFiles.ts
+++ b/packages/node-config-provider/src/fromSharedConfigFiles.ts
@@ -43,7 +43,8 @@ export const fromSharedConfigFiles =
       return configValue;
     } catch (e) {
       throw new CredentialsProviderError(
-        e.message || `Not found in config files w/ profile [${profile}]: ${getSelectorName(configSelector.toString())}`
+        e.message || `Not found in config files w/ profile [${profile}]: ${getSelectorName(configSelector.toString())}`,
+        { logger: init.logger }
       );
     }
   };
diff --git a/packages/property-provider/src/CredentialsProviderError.spec.ts b/packages/property-provider/src/CredentialsProviderError.spec.ts
index 7b6f9caa550..83a7fe0f473 100644
--- a/packages/property-provider/src/CredentialsProviderError.spec.ts
+++ b/packages/property-provider/src/CredentialsProviderError.spec.ts
@@ -2,22 +2,22 @@ import { CredentialsProviderError } from "./CredentialsProviderError";
 import { ProviderError } from "./ProviderError";
 
 describe(CredentialsProviderError.name, () => {
-  afterAll(() => {
-    CredentialsProviderError.logLimit = 100;
-  });
-
   it("should be named as CredentialsProviderError", () => {
     expect(new CredentialsProviderError("PANIC").name).toBe("CredentialsProviderError");
   });
 
-  it("should log up to its logLimit", () => {
-    CredentialsProviderError.logLimit = 5;
-
-    Array.from({ length: CredentialsProviderError.logLimit + 2 }).forEach(() => {
-      new CredentialsProviderError("PANIC");
-    });
+  it("should use logger.trace if provided", () => {
+    const logger = {
+      info: jest.fn(),
+      warn: jest.fn(),
+      error: jest.fn(),
+      debug: jest.fn(),
+      trace: jest.fn(),
+    };
+    new CredentialsProviderError("PANIC", { logger });
 
-    expect(CredentialsProviderError.log.length).toEqual(CredentialsProviderError.logLimit);
+    expect(logger.debug).not.toHaveBeenCalled();
+    expect(logger.trace).toHaveBeenCalled();
   });
 
   describe.each([Error, ProviderError, CredentialsProviderError])("should be instanceof %p", (classConstructor) => {
diff --git a/packages/property-provider/src/CredentialsProviderError.ts b/packages/property-provider/src/CredentialsProviderError.ts
index 50c1e8b8b14..5431cbf8256 100644
--- a/packages/property-provider/src/CredentialsProviderError.ts
+++ b/packages/property-provider/src/CredentialsProviderError.ts
@@ -1,5 +1,15 @@
+import { Logger } from "@smithy/types";
+
 import { ProviderError } from "./ProviderError";
 
+/**
+ * @public
+ */
+export type CredentialsProviderErrorOptionsType = {
+  tryNextLink?: boolean | undefined;
+  logger?: Logger;
+};
+
 /**
  * @public
  *
@@ -12,18 +22,35 @@ import { ProviderError } from "./ProviderError";
  * ensures the chain will stop if an entirely unexpected error is encountered.
  */
 export class CredentialsProviderError extends ProviderError {
-  public static log: string[] = [];
-  public static logLimit = 100;
   public name = "CredentialsProviderError";
-  public constructor(
-    message: string,
-    public readonly tryNextLink: boolean = true
-  ) {
+  public readonly tryNextLink: boolean = true;
+
+  /**
+   * @deprecated constructor should be given a logger.
+   */
+  public constructor(message: string);
+  /**
+   * @deprecated constructor should be given a logger.
+   */
+  public constructor(message: string, tryNextLink?: boolean | undefined);
+  /**
+   * This signature is preferred for logging capability.
+   */
+  public constructor(message: string, options: CredentialsProviderErrorOptionsType);
+  public constructor(message: string, options: boolean | CredentialsProviderErrorOptionsType = true) {
+    let logger: Logger | undefined;
+    let tryNextLink: boolean = true;
+
+    if (typeof options === "boolean") {
+      logger = undefined;
+      tryNextLink = options;
+    } else if (options != null && typeof options === "object") {
+      logger = options.logger;
+      tryNextLink = options.tryNextLink ?? true;
+    }
     super(message, tryNextLink);
+
     Object.setPrototypeOf(this, CredentialsProviderError.prototype);
-    CredentialsProviderError.log.push(`${new Date().toISOString()} ${tryNextLink ? "->" : "(!)"} ${message}`);
-    while (CredentialsProviderError.log.length > CredentialsProviderError.logLimit) {
-      CredentialsProviderError.log.shift();
-    }
+    logger?.trace?.(`${new Date().toISOString()} ${tryNextLink ? "->" : "(!)"} ${message}`);
   }
 }
diff --git a/packages/shared-ini-file-loader/src/loadSharedConfigFiles.ts b/packages/shared-ini-file-loader/src/loadSharedConfigFiles.ts
index 3b16fcd408f..9bf6336e877 100644
--- a/packages/shared-ini-file-loader/src/loadSharedConfigFiles.ts
+++ b/packages/shared-ini-file-loader/src/loadSharedConfigFiles.ts
@@ -1,4 +1,4 @@
-import { SharedConfigFiles } from "@smithy/types";
+import { Logger, SharedConfigFiles } from "@smithy/types";
 
 import { getConfigData } from "./getConfigData";
 import { getConfigFilepath } from "./getConfigFilepath";
@@ -26,6 +26,11 @@ export interface SharedConfigInit {
    * property is set, the provider will always reload any configuration files loaded before.
    */
   ignoreCache?: boolean;
+
+  /**
+   * For credential resolution trace logging.
+   */
+  logger?: Logger;
 }
 
 const swallowError = () => ({});

From da6e5748c3b8cd946441e3c1bc5b256dc962e37d Mon Sep 17 00:00:00 2001
From: George Fu <kuhe@users.noreply.github.com>
Date: Fri, 24 May 2024 15:21:38 +0000
Subject: [PATCH 3/6] var name clarity

---
 .../src/fromInstanceMetadata.ts                     | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/packages/credential-provider-imds/src/fromInstanceMetadata.ts b/packages/credential-provider-imds/src/fromInstanceMetadata.ts
index 0d9caf8dee3..b1fca9f047e 100644
--- a/packages/credential-provider-imds/src/fromInstanceMetadata.ts
+++ b/packages/credential-provider-imds/src/fromInstanceMetadata.ts
@@ -25,9 +25,12 @@ const X_AWS_EC2_METADATA_TOKEN = "x-aws-ec2-metadata-token";
  * Instance Metadata Service
  */
 export const fromInstanceMetadata = (init: RemoteProviderInit = {}): Provider<InstanceMetadataCredentials> =>
-  staticStabilityProvider(getInstanceImdsProvider(init), { logger: init.logger });
+  staticStabilityProvider(getInstanceMetadataProvider(init), { logger: init.logger });
 
-const getInstanceImdsProvider = (init: RemoteProviderInit) => {
+/**
+ * @internal
+ */
+const getInstanceMetadataProvider = (init: RemoteProviderInit = {}) => {
   // when set to true, metadata service will not fetch token
   let disableFetchToken = false;
   const { logger, profile } = init;
@@ -154,7 +157,7 @@ const getMetadataToken = async (options: RequestOptions) =>
 const getProfile = async (options: RequestOptions) => (await httpRequest({ ...options, path: IMDS_PATH })).toString();
 
 const getCredentialsFromProfile = async (profile: string, options: RequestOptions, init: RemoteProviderInit) => {
-  const credsResponse = JSON.parse(
+  const credentialsResponse = JSON.parse(
     (
       await httpRequest({
         ...options,
@@ -163,11 +166,11 @@ const getCredentialsFromProfile = async (profile: string, options: RequestOption
     ).toString()
   );
 
-  if (!isImdsCredentials(credsResponse)) {
+  if (!isImdsCredentials(credentialsResponse)) {
     throw new CredentialsProviderError("Invalid response received from instance metadata service.", {
       logger: init.logger,
     });
   }
 
-  return fromImdsCredentials(credsResponse);
+  return fromImdsCredentials(credentialsResponse);
 };

From 9d8a58976280b78588948187ffebbdf4699ae89f Mon Sep 17 00:00:00 2001
From: George Fu <kuhe@users.noreply.github.com>
Date: Fri, 24 May 2024 16:09:54 +0000
Subject: [PATCH 4/6] test fixes

---
 .../node-config-provider/src/fromEnv.spec.ts  | 29 ++++++-----
 .../src/fromSharedConfigFiles.spec.ts         | 48 +++++++++---------
 .../src/getSelectorName.ts                    | 14 ++++--
 .../src/CredentialsProviderError.spec.ts      | 28 +++++++++--
 .../src/CredentialsProviderError.ts           | 33 +++---------
 .../property-provider/src/ProviderError.ts    | 50 ++++++++++++++++---
 .../src/TokenProviderError.ts                 | 22 +++++---
 7 files changed, 137 insertions(+), 87 deletions(-)

diff --git a/packages/node-config-provider/src/fromEnv.spec.ts b/packages/node-config-provider/src/fromEnv.spec.ts
index 9ead7ad07f3..2b1508396a9 100644
--- a/packages/node-config-provider/src/fromEnv.spec.ts
+++ b/packages/node-config-provider/src/fromEnv.spec.ts
@@ -4,26 +4,30 @@ import { fromEnv, GetterFromEnv } from "./fromEnv";
 
 describe("fromEnv", () => {
   describe("with env var getter", () => {
-    const envVarName = "ENV_VAR_NAME";
+    const ENV_VAR_NAME = "ENV_VAR_NAME";
 
     // Using Record<string, string | undefined> instead of NodeJS.ProcessEnv, in order to not get type errors in non node environments
-    const envVarGetter: GetterFromEnv<string> = (env: Record<string, string | undefined>) => env[envVarName]!;
-    const envVarValue = process.env[envVarName];
+    const envVarGetter: GetterFromEnv<string> = (env: Record<string, string | undefined>) => env[ENV_VAR_NAME]!;
+    const envVarValue = process.env[ENV_VAR_NAME];
     const mockEnvVarValue = "mockEnvVarValue";
 
-    const getCredentialsProviderError = (getter: GetterFromEnv<string>) =>
-      new CredentialsProviderError(`Cannot load config from environment variables with getter: ${getter}`);
-
     beforeEach(() => {
-      delete process.env[envVarName];
+      delete process.env[ENV_VAR_NAME];
     });
 
     afterAll(() => {
-      process.env[envVarName] = envVarValue;
+      process.env[ENV_VAR_NAME] = envVarValue;
+    });
+
+    describe("CredentialsProviderError", () => {
+      it("is behaving as expected cross-package in jest", () => {
+        expect(new CredentialsProviderError("msg", {}).message).toEqual("msg");
+        expect(new CredentialsProviderError("msg", {}).name).toEqual("CredentialsProviderError");
+      });
     });
 
-    it(`returns string value in '${envVarName}' env var when set`, () => {
-      process.env[envVarName] = mockEnvVarValue;
+    it(`returns string value in '${ENV_VAR_NAME}' env var when set`, () => {
+      process.env[ENV_VAR_NAME] = mockEnvVarValue;
       return expect(fromEnv(envVarGetter)()).resolves.toBe(mockEnvVarValue);
     });
 
@@ -35,9 +39,10 @@ describe("fromEnv", () => {
       return expect(fromEnv(getter)()).resolves.toEqual(value);
     });
 
-    it(`throws when '${envVarName}' env var is not set`, () => {
+    it(`throws when '${ENV_VAR_NAME}' env var is not set`, async () => {
       expect.assertions(1);
-      return expect(fromEnv(envVarGetter)()).rejects.toMatchObject(getCredentialsProviderError(envVarGetter));
+      const error = await fromEnv(envVarGetter)().catch((_) => _);
+      return expect(error).toEqual(new CredentialsProviderError(`Not found in ENV: ENV_VAR_NAME`, {}));
     });
 
     it("throws when the getter function throws", () => {
diff --git a/packages/node-config-provider/src/fromSharedConfigFiles.spec.ts b/packages/node-config-provider/src/fromSharedConfigFiles.spec.ts
index c2b3c2edb7f..3382dc9d947 100644
--- a/packages/node-config-provider/src/fromSharedConfigFiles.spec.ts
+++ b/packages/node-config-provider/src/fromSharedConfigFiles.spec.ts
@@ -10,13 +10,11 @@ jest.mock("@smithy/shared-ini-file-loader", () => ({
 }));
 
 describe("fromSharedConfigFiles", () => {
-  const configKey = "config_key";
-  const configGetter: GetterFromConfig<string> = (profile: Profile) => profile[configKey];
+  const CONFIG_KEY = "config_key";
+  const configGetter: GetterFromConfig<string> = (profile: Profile) => profile[CONFIG_KEY];
 
-  const getCredentialsProviderError = (profile: string, getter: GetterFromConfig<string>) =>
-    new CredentialsProviderError(
-      `Cannot load config for profile ${profile} in SDK configuration files with getter: ${getter}`
-    );
+  const getCredentialsProviderError = (profile: string) =>
+    new CredentialsProviderError(`Not found in config files w/ profile [${profile}]: CONFIG_KEY`, {});
 
   describe("loadedConfig", () => {
     const mockConfigAnswer = "mockConfigAnswer";
@@ -36,21 +34,21 @@ describe("fromSharedConfigFiles", () => {
       {
         message: "returns configValue from default profile",
         iniDataInConfig: {
-          default: { [configKey]: mockConfigAnswer },
+          default: { [CONFIG_KEY]: mockConfigAnswer },
         },
         iniDataInCredentials: {
-          default: { [configKey]: mockCredentialsNotAnswer },
+          default: { [CONFIG_KEY]: mockCredentialsNotAnswer },
         },
         configValueToVerify: mockConfigAnswer,
       },
       {
         message: "returns configValue from designated profile",
         iniDataInConfig: {
-          default: { [configKey]: mockConfigNotAnswer },
-          foo: { [configKey]: mockConfigAnswer },
+          default: { [CONFIG_KEY]: mockConfigNotAnswer },
+          foo: { [CONFIG_KEY]: mockConfigAnswer },
         },
         iniDataInCredentials: {
-          foo: { [configKey]: mockCredentialsNotAnswer },
+          foo: { [CONFIG_KEY]: mockCredentialsNotAnswer },
         },
         profile: "foo",
         configValueToVerify: mockConfigAnswer,
@@ -58,11 +56,11 @@ describe("fromSharedConfigFiles", () => {
       {
         message: "returns configValue from credentials file if preferred",
         iniDataInConfig: {
-          default: { [configKey]: mockConfigNotAnswer },
-          foo: { [configKey]: mockConfigNotAnswer },
+          default: { [CONFIG_KEY]: mockConfigNotAnswer },
+          foo: { [CONFIG_KEY]: mockConfigNotAnswer },
         },
         iniDataInCredentials: {
-          foo: { [configKey]: mockCredentialsAnswer },
+          foo: { [CONFIG_KEY]: mockCredentialsAnswer },
         },
         profile: "foo",
         preferredFile: "credentials",
@@ -71,7 +69,7 @@ describe("fromSharedConfigFiles", () => {
       {
         message: "returns configValue from config file if preferred credentials file doesn't contain config",
         iniDataInConfig: {
-          foo: { [configKey]: mockConfigAnswer },
+          foo: { [CONFIG_KEY]: mockConfigAnswer },
         },
         iniDataInCredentials: {},
         configValueToVerify: mockConfigAnswer,
@@ -82,7 +80,7 @@ describe("fromSharedConfigFiles", () => {
         message: "returns configValue from credential file if preferred config file doesn't contain config",
         iniDataInConfig: {},
         iniDataInCredentials: {
-          foo: { [configKey]: mockCredentialsAnswer },
+          foo: { [CONFIG_KEY]: mockCredentialsAnswer },
         },
         configValueToVerify: mockCredentialsAnswer,
         profile: "foo",
@@ -93,14 +91,14 @@ describe("fromSharedConfigFiles", () => {
       {
         message: "rejects if default profile is not present and profile value is not passed",
         iniDataInConfig: {
-          foo: { [configKey]: mockConfigNotAnswer },
+          foo: { [CONFIG_KEY]: mockConfigNotAnswer },
         },
         iniDataInCredentials: {},
       },
       {
         message: "rejects if designated profile is not present",
         iniDataInConfig: {
-          default: { [configKey]: mockConfigNotAnswer },
+          default: { [CONFIG_KEY]: mockConfigNotAnswer },
         },
         iniDataInCredentials: {},
         profile: "foo",
@@ -129,8 +127,8 @@ describe("fromSharedConfigFiles", () => {
           credentialsFile: iniDataInCredentials,
         });
         (getProfileName as jest.Mock).mockReturnValueOnce(profile ?? "default");
-        return expect(fromSharedConfigFiles(configGetter, { profile, preferredFile })()).rejects.toMatchObject(
-          getCredentialsProviderError(profile ?? "default", configGetter)
+        return expect(fromSharedConfigFiles(configGetter, { profile, preferredFile })()).rejects.toEqual(
+          getCredentialsProviderError(profile ?? "default")
         );
       });
     });
@@ -144,18 +142,18 @@ describe("fromSharedConfigFiles", () => {
         configFile: {},
         credentialsFile: {},
       });
-      return expect(fromSharedConfigFiles(failGetter)()).rejects.toMatchObject(new CredentialsProviderError(message));
+      return expect(fromSharedConfigFiles(failGetter)()).rejects.toEqual(new CredentialsProviderError(message));
     });
   });
 
   describe("profile", () => {
     const loadedConfigData = {
       configFile: {
-        default: { [configKey]: "configFileDefault" },
-        foo: { [configKey]: "configFileFoo" },
+        default: { [CONFIG_KEY]: "configFileDefault" },
+        foo: { [CONFIG_KEY]: "configFileFoo" },
       },
       credentialsFile: {
-        default: { [configKey]: "credentialsFileDefault" },
+        default: { [CONFIG_KEY]: "credentialsFileDefault" },
       },
     };
 
@@ -166,7 +164,7 @@ describe("fromSharedConfigFiles", () => {
     it.each(["foo", "default"])("returns config value from %s profile", (profile) => {
       (getProfileName as jest.Mock).mockReturnValueOnce(profile);
       return expect(fromSharedConfigFiles(configGetter)()).resolves.toBe(
-        loadedConfigData.configFile[profile][configKey]
+        loadedConfigData.configFile[profile][CONFIG_KEY]
       );
     });
   });
diff --git a/packages/node-config-provider/src/getSelectorName.ts b/packages/node-config-provider/src/getSelectorName.ts
index 58a6fa68422..7aa405c20ef 100644
--- a/packages/node-config-provider/src/getSelectorName.ts
+++ b/packages/node-config-provider/src/getSelectorName.ts
@@ -7,9 +7,13 @@
  * @returns constant value used within the function.
  */
 export function getSelectorName(functionString: string): string {
-  const constants = new Set(Array.from(functionString.match(/([A-Z_]){3,}/g) ?? []));
-  constants.delete("CONFIG");
-  constants.delete("CONFIG_PREFIX_SEPARATOR");
-  constants.delete("ENV");
-  return [...constants].join(", ");
+  try {
+    const constants = new Set(Array.from(functionString.match(/([A-Z_]){3,}/g) ?? []));
+    constants.delete("CONFIG");
+    constants.delete("CONFIG_PREFIX_SEPARATOR");
+    constants.delete("ENV");
+    return [...constants].join(", ");
+  } catch (e) {
+    return functionString;
+  }
 }
diff --git a/packages/property-provider/src/CredentialsProviderError.spec.ts b/packages/property-provider/src/CredentialsProviderError.spec.ts
index 83a7fe0f473..f3deda45d8b 100644
--- a/packages/property-provider/src/CredentialsProviderError.spec.ts
+++ b/packages/property-provider/src/CredentialsProviderError.spec.ts
@@ -2,11 +2,31 @@ import { CredentialsProviderError } from "./CredentialsProviderError";
 import { ProviderError } from "./ProviderError";
 
 describe(CredentialsProviderError.name, () => {
-  it("should be named as CredentialsProviderError", () => {
+  it("should be named CredentialsProviderError", () => {
     expect(new CredentialsProviderError("PANIC").name).toBe("CredentialsProviderError");
   });
 
-  it("should use logger.trace if provided", () => {
+  it("should have a non-enumerable message like the base Error class", () => {
+    expect(new CredentialsProviderError("PANIC", {}).message).toBe("PANIC");
+
+    expect(
+      {
+        ...new CredentialsProviderError("PANIC", {}),
+      }.message
+    ).toBe(undefined);
+  });
+
+  it("should have an enumerable tryNextLink and logger like the base Error class", () => {
+    expect(new CredentialsProviderError("PANIC", {}).tryNextLink).toBe(true);
+
+    expect(
+      {
+        ...new CredentialsProviderError("PANIC", { tryNextLink: false }),
+      }.tryNextLink
+    ).toBe(false);
+  });
+
+  it("should use logger.debug if provided", () => {
     const logger = {
       info: jest.fn(),
       warn: jest.fn(),
@@ -16,8 +36,8 @@ describe(CredentialsProviderError.name, () => {
     };
     new CredentialsProviderError("PANIC", { logger });
 
-    expect(logger.debug).not.toHaveBeenCalled();
-    expect(logger.trace).toHaveBeenCalled();
+    expect(logger.debug).toHaveBeenCalled();
+    expect(logger.trace).not.toHaveBeenCalled();
   });
 
   describe.each([Error, ProviderError, CredentialsProviderError])("should be instanceof %p", (classConstructor) => {
diff --git a/packages/property-provider/src/CredentialsProviderError.ts b/packages/property-provider/src/CredentialsProviderError.ts
index 5431cbf8256..62a2dfcc020 100644
--- a/packages/property-provider/src/CredentialsProviderError.ts
+++ b/packages/property-provider/src/CredentialsProviderError.ts
@@ -1,14 +1,6 @@
 import { Logger } from "@smithy/types";
 
-import { ProviderError } from "./ProviderError";
-
-/**
- * @public
- */
-export type CredentialsProviderErrorOptionsType = {
-  tryNextLink?: boolean | undefined;
-  logger?: Logger;
-};
+import { ProviderError, ProviderErrorOptionsType } from "./ProviderError";
 
 /**
  * @public
@@ -22,8 +14,7 @@ export type CredentialsProviderErrorOptionsType = {
  * ensures the chain will stop if an entirely unexpected error is encountered.
  */
 export class CredentialsProviderError extends ProviderError {
-  public name = "CredentialsProviderError";
-  public readonly tryNextLink: boolean = true;
+  name = "CredentialsProviderError";
 
   /**
    * @deprecated constructor should be given a logger.
@@ -32,25 +23,13 @@ export class CredentialsProviderError extends ProviderError {
   /**
    * @deprecated constructor should be given a logger.
    */
-  public constructor(message: string, tryNextLink?: boolean | undefined);
+  public constructor(message: string, tryNextLink: boolean | undefined);
   /**
    * This signature is preferred for logging capability.
    */
-  public constructor(message: string, options: CredentialsProviderErrorOptionsType);
-  public constructor(message: string, options: boolean | CredentialsProviderErrorOptionsType = true) {
-    let logger: Logger | undefined;
-    let tryNextLink: boolean = true;
-
-    if (typeof options === "boolean") {
-      logger = undefined;
-      tryNextLink = options;
-    } else if (options != null && typeof options === "object") {
-      logger = options.logger;
-      tryNextLink = options.tryNextLink ?? true;
-    }
-    super(message, tryNextLink);
-
+  public constructor(message: string, options: ProviderErrorOptionsType);
+  public constructor(message: string, options: boolean | ProviderErrorOptionsType = true) {
+    super(message, options as ProviderErrorOptionsType);
     Object.setPrototypeOf(this, CredentialsProviderError.prototype);
-    logger?.trace?.(`${new Date().toISOString()} ${tryNextLink ? "->" : "(!)"} ${message}`);
   }
 }
diff --git a/packages/property-provider/src/ProviderError.ts b/packages/property-provider/src/ProviderError.ts
index 4f1603ebf79..546e9458eee 100644
--- a/packages/property-provider/src/ProviderError.ts
+++ b/packages/property-provider/src/ProviderError.ts
@@ -1,3 +1,13 @@
+import { Logger } from "@smithy/types";
+
+/**
+ * @public
+ */
+export type ProviderErrorOptionsType = {
+  tryNextLink?: boolean | undefined;
+  logger?: Logger;
+};
+
 /**
  * @public
  *
@@ -11,15 +21,41 @@
  */
 export class ProviderError extends Error {
   name = "ProviderError";
-  constructor(
-    message: string,
-    public readonly tryNextLink: boolean = true
-  ) {
+  public readonly tryNextLink: boolean;
+
+  /**
+   * @deprecated constructor should be given a logger.
+   */
+  public constructor(message: string);
+  /**
+   * @deprecated constructor should be given a logger.
+   */
+  public constructor(message: string, tryNextLink: boolean | undefined);
+  /**
+   * This signature is preferred for logging capability.
+   */
+  public constructor(message: string, options: ProviderErrorOptionsType);
+  public constructor(message: string, options: boolean | ProviderErrorOptionsType = true) {
+    let logger: Logger | undefined;
+    let tryNextLink: boolean = true;
+
+    if (typeof options === "boolean") {
+      logger = undefined;
+      tryNextLink = options;
+    } else if (options != null && typeof options === "object") {
+      logger = options.logger;
+      tryNextLink = options.tryNextLink ?? true;
+    }
     super(message);
-    // Remove once we stop targetting ES5.
+    this.tryNextLink = tryNextLink;
     Object.setPrototypeOf(this, ProviderError.prototype);
+    logger?.debug?.(`${this.constructor?.name} ${tryNextLink ? "->" : "(!)"} ${message}`);
   }
-  static from(error: Error, tryNextLink = true): ProviderError {
-    return Object.assign(new this(error.message, tryNextLink), error);
+
+  /**
+   * @deprecated use new operator.
+   */
+  static from(error: Error, options: boolean | ProviderErrorOptionsType = true): ProviderError {
+    return Object.assign(new this(error.message, options as ProviderErrorOptionsType), error);
   }
 }
diff --git a/packages/property-provider/src/TokenProviderError.ts b/packages/property-provider/src/TokenProviderError.ts
index b0c461e85c8..9d3ddcf763f 100644
--- a/packages/property-provider/src/TokenProviderError.ts
+++ b/packages/property-provider/src/TokenProviderError.ts
@@ -1,4 +1,4 @@
-import { ProviderError } from "./ProviderError";
+import { ProviderError, ProviderErrorOptionsType } from "./ProviderError";
 
 /**
  * @public
@@ -13,12 +13,20 @@ import { ProviderError } from "./ProviderError";
  */
 export class TokenProviderError extends ProviderError {
   name = "TokenProviderError";
-  constructor(
-    message: string,
-    public readonly tryNextLink: boolean = true
-  ) {
-    super(message, tryNextLink);
-    // Remove once we stop targetting ES5.
+  /**
+   * @deprecated constructor should be given a logger.
+   */
+  public constructor(message: string);
+  /**
+   * @deprecated constructor should be given a logger.
+   */
+  public constructor(message: string, tryNextLink: boolean | undefined);
+  /**
+   * This signature is preferred for logging capability.
+   */
+  public constructor(message: string, options: ProviderErrorOptionsType);
+  public constructor(message: string, options: boolean | ProviderErrorOptionsType = true) {
+    super(message, options as ProviderErrorOptionsType);
     Object.setPrototypeOf(this, TokenProviderError.prototype);
   }
 }

From 454a37cf4a45ce4a2b69467fa4582afe41519a50 Mon Sep 17 00:00:00 2001
From: George Fu <kuhe@users.noreply.github.com>
Date: Fri, 24 May 2024 16:15:19 +0000
Subject: [PATCH 5/6] formatting

---
 .../property-provider/src/CredentialsProviderError.ts     | 8 ++++++--
 packages/property-provider/src/ProviderError.ts           | 2 +-
 packages/property-provider/src/TokenProviderError.ts      | 7 +++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/packages/property-provider/src/CredentialsProviderError.ts b/packages/property-provider/src/CredentialsProviderError.ts
index 62a2dfcc020..75067619f95 100644
--- a/packages/property-provider/src/CredentialsProviderError.ts
+++ b/packages/property-provider/src/CredentialsProviderError.ts
@@ -1,5 +1,3 @@
-import { Logger } from "@smithy/types";
-
 import { ProviderError, ProviderErrorOptionsType } from "./ProviderError";
 
 /**
@@ -17,17 +15,23 @@ export class CredentialsProviderError extends ProviderError {
   name = "CredentialsProviderError";
 
   /**
+   * @override
    * @deprecated constructor should be given a logger.
    */
   public constructor(message: string);
   /**
+   * @override
    * @deprecated constructor should be given a logger.
    */
   public constructor(message: string, tryNextLink: boolean | undefined);
   /**
+   * @override
    * This signature is preferred for logging capability.
    */
   public constructor(message: string, options: ProviderErrorOptionsType);
+  /**
+   * @override
+   */
   public constructor(message: string, options: boolean | ProviderErrorOptionsType = true) {
     super(message, options as ProviderErrorOptionsType);
     Object.setPrototypeOf(this, CredentialsProviderError.prototype);
diff --git a/packages/property-provider/src/ProviderError.ts b/packages/property-provider/src/ProviderError.ts
index 546e9458eee..f239daf5c90 100644
--- a/packages/property-provider/src/ProviderError.ts
+++ b/packages/property-provider/src/ProviderError.ts
@@ -49,7 +49,7 @@ export class ProviderError extends Error {
     super(message);
     this.tryNextLink = tryNextLink;
     Object.setPrototypeOf(this, ProviderError.prototype);
-    logger?.debug?.(`${this.constructor?.name} ${tryNextLink ? "->" : "(!)"} ${message}`);
+    logger?.debug?.(`@smithy/property-provider ${tryNextLink ? "->" : "(!)"} ${message}`);
   }
 
   /**
diff --git a/packages/property-provider/src/TokenProviderError.ts b/packages/property-provider/src/TokenProviderError.ts
index 9d3ddcf763f..6799830c7f0 100644
--- a/packages/property-provider/src/TokenProviderError.ts
+++ b/packages/property-provider/src/TokenProviderError.ts
@@ -13,18 +13,25 @@ import { ProviderError, ProviderErrorOptionsType } from "./ProviderError";
  */
 export class TokenProviderError extends ProviderError {
   name = "TokenProviderError";
+
   /**
+   * @override
    * @deprecated constructor should be given a logger.
    */
   public constructor(message: string);
   /**
+   * @override
    * @deprecated constructor should be given a logger.
    */
   public constructor(message: string, tryNextLink: boolean | undefined);
   /**
+   * @override
    * This signature is preferred for logging capability.
    */
   public constructor(message: string, options: ProviderErrorOptionsType);
+  /**
+   * @override
+   */
   public constructor(message: string, options: boolean | ProviderErrorOptionsType = true) {
     super(message, options as ProviderErrorOptionsType);
     Object.setPrototypeOf(this, TokenProviderError.prototype);

From d514abb6a1853f5fcc26bc22686f987081d0fd0c Mon Sep 17 00:00:00 2001
From: George Fu <kuhe@users.noreply.github.com>
Date: Wed, 29 May 2024 13:01:01 -0400
Subject: [PATCH 6/6] set internal tag on getSelectorName

---
 packages/node-config-provider/src/getSelectorName.ts | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/packages/node-config-provider/src/getSelectorName.ts b/packages/node-config-provider/src/getSelectorName.ts
index 7aa405c20ef..7acc4c9211b 100644
--- a/packages/node-config-provider/src/getSelectorName.ts
+++ b/packages/node-config-provider/src/getSelectorName.ts
@@ -1,7 +1,7 @@
 /**
  * Attempts to extract the name of the variable that the functional selector is looking for.
  * Improves readability over the raw Function.toString() value.
- *
+ * @internal
  * @param functionString - function's string representation.
  *
  * @returns constant value used within the function.