chore: add logging for CredentialsProviderError (#1290)

* chore: add logging for CredentialsProviderError * use options object * var name clarity * test fixes * formatting * set internal tag on getSelectorName
smithy-lang · May 29, 2024 · 1cdd3be · 1cdd3be
1 parent 764047e
commit 1cdd3be
Show file tree

Hide file tree

Showing 13 changed files with 238 additions and 87 deletions.
diff --git a/.changeset/late-glasses-jam.md 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/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
@@ -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;
@@ -47,7 +50,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 +102,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,8 +156,8 @@ 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 credsResponse = JSON.parse(
+const getCredentialsFromProfile = async (profile: string, options: RequestOptions, init: RemoteProviderInit) => {
+  const credentialsResponse = JSON.parse(
     (
       await httpRequest({
         ...options,
@@ -162,9 +166,11 @@ const getCredentialsFromProfile = async (profile: string, options: RequestOption
     ).toString()
   );
 
-  if (!isImdsCredentials(credsResponse)) {
-    throw new CredentialsProviderError("Invalid response received from instance metadata service.");
+  if (!isImdsCredentials(credentialsResponse)) {
+    throw new CredentialsProviderError("Invalid response received from instance metadata service.", {
+      logger: init.logger,
+    });
   }
 
-  return fromImdsCredentials(credsResponse);
+  return fromImdsCredentials(credentialsResponse);
 };
diff --git 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/fromEnv.ts b/packages/node-config-provider/src/fromEnv.ts
@@ -1,5 +1,7 @@
 import { CredentialsProviderError } from "@smithy/property-provider";
-import { Provider } from "@smithy/types";
+import { Logger, 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;
@@ -9,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);
@@ -19,7 +21,8 @@ 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())}`,
+        { logger }
       );
     }
   };
diff --git 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,33 +34,33 @@ 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,
       },
       {
         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/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,8 @@ 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())}`,
+        { logger: init.logger }
       );
     }
   };
diff --git a/packages/node-config-provider/src/getSelectorName.ts b/packages/node-config-provider/src/getSelectorName.ts
@@ -0,0 +1,19 @@
+/**
+ * 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.
+ */
+export function getSelectorName(functionString: string): string {
+  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;
+  }
+}