Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: add logging for CredentialsProviderError #1290

Merged
merged 6 commits into from
May 29, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions .changeset/two-weeks-provide.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
"@smithy/node-config-provider": patch
"@smithy/property-provider": patch
---

add logging capability for CredentialsProviderError
4 changes: 3 additions & 1 deletion packages/node-config-provider/src/fromEnv.ts
Original file line number Diff line number Diff line change
@@ -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;

Expand All @@ -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}`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we were previously logging the Function.toString() value, which is quite unreadable...

e.message || `Not found in ENV: ${getSelectorName(envVarSelector.toString())}`
);
}
};
5 changes: 3 additions & 2 deletions packages/node-config-provider/src/fromSharedConfigFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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())}`
);
}
};
15 changes: 15 additions & 0 deletions packages/node-config-provider/src/getSelectorName.ts
Original file line number Diff line number Diff line change
@@ -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.
*
kuhe marked this conversation as resolved.
Show resolved Hide resolved
* @param functionString - function's string representation.
*
* @returns constant value used within the function.
*/
export function getSelectorName(functionString: string): string {
kuhe marked this conversation as resolved.
Show resolved Hide resolved
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(", ");
}
14 changes: 14 additions & 0 deletions packages/property-provider/src/CredentialsProviderError.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
11 changes: 8 additions & 3 deletions packages/property-provider/src/CredentialsProviderError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}
}
}
Loading