Added runtime validations for the metrics and dimensions utility functions

sdangol · sdangol · commit f673c07f5869 · 2025-07-16T18:01:26.000+01:00
diff --git a/packages/commons/src/index.ts b/packages/commons/src/index.ts
@@ -22,6 +22,7 @@ export {
   isRecord,
   isStrictEqual,
   isString,
+  isStringUndefinedNullEmpty,
   isTruthy,
 } from './typeUtils.js';
 export { Utility } from './Utility.js';
diff --git a/packages/commons/src/typeUtils.ts b/packages/commons/src/typeUtils.ts
@@ -154,6 +154,28 @@ const isNullOrUndefined = (value: unknown): value is null | undefined => {
   return isNull(value) || Object.is(value, undefined);
 };
 
+/**
+ * Check if string is undefined, null, empty.
+ *
+ * @example
+ * ```typescript
+ * import { isStringUndefinedNullEmpty } from '@aws-lambda-powertools/commons/typeUtils';
+ *
+ * const value = 'foo';
+ * if (isStringUndefinedNullEmpty(value)) {
+ *   // value is either undefined, null, or an empty string
+ * }
+ * ```
+ *
+ * @param value The value to check
+ */
+const isStringUndefinedNullEmpty = (value: unknown) => {
+  if (isNullOrUndefined(value)) return true;
+  if (!isString(value)) return true;
+  if (value.trim().length === 0) return true;
+  return false;
+};
+
 /**
  * Get the type of a value as a string.
  *
@@ -314,6 +336,7 @@ export {
   isTruthy,
   isNull,
   isNullOrUndefined,
+  isStringUndefinedNullEmpty,
   getType,
   isStrictEqual,
 };
diff --git a/packages/commons/tests/unit/typeUtils.test.ts b/packages/commons/tests/unit/typeUtils.test.ts
@@ -8,6 +8,7 @@ import {
   isRecord,
   isStrictEqual,
   isString,
+  isStringUndefinedNullEmpty,
   isTruthy,
 } from '../../src/index.js';
 
@@ -119,6 +120,38 @@ describe('Functions: typeUtils', () => {
     });
   });
 
+  describe('Function: isStringUndefinedNullEmpty', () => {
+    it('returns true if input is undefined', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(undefined)).toBe(true);
+    });
+
+    it('returns true if input is null', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(null)).toBe(true);
+    });
+
+    it('returns true if input is an empty string', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty('')).toBe(true);
+    });
+
+    it('returns true if input is a whitespace', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(' ')).toBe(true);
+    });
+
+    it('returns true if input is not a string', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty(1)).toBe(true);
+    });
+
+    it('returns false if input is not undefined, null, or an empty string', () => {
+      // Act & Assess
+      expect(isStringUndefinedNullEmpty('test')).toBe(false);
+    });
+  });
+
   describe('Function: isNumber', () => {
     it('returns true when the passed value is a number', () => {
       // Prepare
diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts
@@ -1,5 +1,11 @@
 import { Console } from 'node:console';
-import { isIntegerNumber, Utility } from '@aws-lambda-powertools/commons';
+import {
+  isIntegerNumber,
+  isNumber,
+  isString,
+  isStringUndefinedNullEmpty,
+  Utility,
+} from '@aws-lambda-powertools/commons';
 import type {
   GenericLogger,
   HandlerMethodDecorator,
@@ -12,10 +18,12 @@ import {
   EMF_MAX_TIMESTAMP_FUTURE_AGE,
   EMF_MAX_TIMESTAMP_PAST_AGE,
   MAX_DIMENSION_COUNT,
+  MAX_METRIC_NAME_LENGTH,
   MAX_METRIC_VALUES_SIZE,
   MAX_METRICS_SIZE,
   MetricResolution as MetricResolutions,
   MetricUnit as MetricUnits,
+  MIN_METRIC_NAME_LENGTH,
 } from './constants.js';
 import type {
   ConfigServiceInterface,
@@ -238,7 +246,7 @@ class Metrics extends Utility implements MetricsInterface {
    * @param value - The value of the dimension
    */
   public addDimension(name: string, value: string): void {
-    if (!value) {
+    if (isStringUndefinedNullEmpty(name) || isStringUndefinedNullEmpty(value)) {
       this.#logger.warn(
         `The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
       );
@@ -275,7 +283,10 @@ class Metrics extends Utility implements MetricsInterface {
   public addDimensions(dimensions: Dimensions): void {
     const newDimensionSet: Dimensions = {};
     for (const [key, value] of Object.entries(dimensions)) {
-      if (!value) {
+      if (
+        isStringUndefinedNullEmpty(key) ||
+        isStringUndefinedNullEmpty(value)
+      ) {
         this.#logger.warn(
           `The dimension ${key} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
         );
@@ -1067,6 +1078,24 @@ class Metrics extends Utility implements MetricsInterface {
     value: number,
     resolution: MetricResolution
   ): void {
+    if (!isString(name)) throw new Error(`${name} is not a valid string`);
+    if (
+      name.length < MIN_METRIC_NAME_LENGTH ||
+      name.length > MAX_METRIC_NAME_LENGTH
+    )
+      throw new RangeError(
+        `The metric name should be between ${MIN_METRIC_NAME_LENGTH} and ${MAX_METRIC_NAME_LENGTH} characters`
+      );
+    if (!isNumber(value)) throw new Error(`${value} is not a valid number`);
+    if (!Object.values(MetricUnits).includes(unit))
+      throw new Error(
+        `Invalid metric unit '${unit}', expected either option: ${Object.values(MetricUnits).join(',')}`
+      );
+    if (!Object.values(MetricResolutions).includes(resolution))
+      throw new Error(
+        `Invalid metric resolution '${resolution}', expected either option: ${Object.values(MetricResolutions).join(',')}`
+      );
+
     if (Object.keys(this.storedMetrics).length >= MAX_METRICS_SIZE) {
       this.publishStoredMetrics();
     }
diff --git a/packages/metrics/src/constants.ts b/packages/metrics/src/constants.ts
@@ -6,6 +6,14 @@ const COLD_START_METRIC = 'ColdStart';
  * The default namespace for metrics.
  */
 const DEFAULT_NAMESPACE = 'default_namespace';
+/**
+ * The minimum length constraint of the metric name
+ */
+const MIN_METRIC_NAME_LENGTH = 1;
+/**
+ * The maximum length constraint of the metric name
+ */
+const MAX_METRIC_NAME_LENGTH = 255;
 /**
  * The maximum number of metrics that can be emitted in a single EMF blob.
  */
@@ -78,6 +86,8 @@ const MetricResolution = {
 export {
   COLD_START_METRIC,
   DEFAULT_NAMESPACE,
+  MIN_METRIC_NAME_LENGTH,
+  MAX_METRIC_NAME_LENGTH,
   MAX_METRICS_SIZE,
   MAX_METRIC_VALUES_SIZE,
   MAX_DIMENSION_COUNT,
diff --git a/packages/metrics/tests/unit/creatingMetrics.test.ts b/packages/metrics/tests/unit/creatingMetrics.test.ts
@@ -1,8 +1,10 @@
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 import {
   DEFAULT_NAMESPACE,
+  MAX_METRIC_NAME_LENGTH,
   MAX_METRICS_SIZE,
   MetricResolution,
+  MIN_METRIC_NAME_LENGTH,
 } from '../../src/constants.js';
 import { Metrics, MetricUnit } from '../../src/index.js';
 
@@ -267,4 +269,78 @@ describe('Creating metrics', () => {
       })
     );
   });
+
+  it('throws when an invalid metric name is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    // @ts-expect-error - Testing runtime behavior with non-numeric metric value
+    expect(() => metrics.addMetric(1, MetricUnit.Count, 1)).toThrowError(
+      '1 is not a valid string'
+    );
+  });
+
+  it('throws when an empty string is passed in the metric name', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() => metrics.addMetric('', MetricUnit.Count, 1)).toThrowError(
+      `The metric name should be between ${MIN_METRIC_NAME_LENGTH} and ${MAX_METRIC_NAME_LENGTH} characters`
+    );
+  });
+
+  it(`throws when a string of more than ${MAX_METRIC_NAME_LENGTH} characters is passed in the metric name`, () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() =>
+      metrics.addMetric(
+        'Lorem ipsum dolor sit amet, consectetuer adipiscing elit. Aenean commodo ligula eget dolor. Aenean massa. Cum sociis natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. Donec quam felis, ultricies nec, pellentesque eu, pretium quis,.',
+        MetricUnit.Count,
+        1
+      )
+    ).toThrowError(
+      new RangeError(
+        `The metric name should be between ${MIN_METRIC_NAME_LENGTH} and ${MAX_METRIC_NAME_LENGTH} characters`
+      )
+    );
+  });
+
+  it('throws when a non-numeric metric value is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() =>
+      // @ts-expect-error - Testing runtime behavior with non-numeric metric value
+      metrics.addMetric('test', MetricUnit.Count, 'one')
+    ).toThrowError('one is not a valid number');
+  });
+
+  it('throws when an invalid unit is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    // @ts-expect-error - Testing runtime behavior with invalid metric unit
+    expect(() => metrics.addMetric('test', 'invalid-unit', 1)).toThrowError(
+      `Invalid metric unit 'invalid-unit', expected either option: ${Object.values(MetricUnit).join(',')}`
+    );
+  });
+
+  it('throws when an invalid resolution is passed', () => {
+    // Prepare
+    const metrics = new Metrics();
+
+    // Act & Assess
+    expect(() =>
+      // @ts-expect-error - Testing runtime behavior with invalid metric unit
+      metrics.addMetric('test', MetricUnit.Count, 1, 'invalid-resolution')
+    ).toThrowError(
+      `Invalid metric resolution 'invalid-resolution', expected either option: ${Object.values(MetricResolution).join(',')}`
+    );
+  });
 });
diff --git a/packages/metrics/tests/unit/dimensions.test.ts b/packages/metrics/tests/unit/dimensions.test.ts
@@ -445,26 +445,32 @@ describe('Working with dimensions', () => {
   });
 
   it.each([
-    { value: undefined, name: 'undefined' },
-    { value: null, name: 'null' },
+    { value: undefined, name: 'valid-name' },
+    { value: null, name: 'valid-name' },
     {
       value: '',
-      name: 'empty string',
+      name: 'valid-name',
     },
-  ])('skips invalid dimension values ($name)', ({ value }) => {
+    { value: 'valid-value', name: undefined },
+    { value: 'valid-value', name: null },
+    {
+      value: 'valid-value',
+      name: '',
+    },
+  ])('skips invalid dimension values ($name)', ({ value, name }) => {
     // Prepare
     const metrics = new Metrics({
       singleMetric: true,
       namespace: DEFAULT_NAMESPACE,
     });
 
     // Act & Assess
-    metrics.addDimension('test', value as string);
+    metrics.addDimension(name as string, value as string);
     metrics.addMetric('test', MetricUnit.Count, 1);
 
     expect(console.warn).toHaveBeenCalledTimes(1);
     expect(console.warn).toHaveBeenCalledWith(
-      `The dimension test doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
+      `The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
     );
     expect(console.log).toHaveEmittedEMFWith(
       expect.not.objectContaining({ test: value })
@@ -475,37 +481,44 @@ describe('Working with dimensions', () => {
   });
 
   it.each([
-    { value: undefined, name: 'undefined' },
-    { value: null, name: 'null' },
+    { value: undefined, name: 'valid-name' },
+    { value: null, name: 'valid-name' },
     {
       value: '',
-      name: 'empty string',
+      name: 'valid-name',
     },
-  ])('skips invalid dimension values in addDimensions ($name)', ({ value }) => {
-    // Prepare
-    const metrics = new Metrics({
-      singleMetric: true,
-      namespace: DEFAULT_NAMESPACE,
-    });
-
-    // Act & Assess
-    metrics.addDimensions({
-      validDimension: 'valid',
-      invalidDimension: value as string,
-    });
-    metrics.addMetric('test', MetricUnit.Count, 1);
-    metrics.publishStoredMetrics();
-
-    expect(console.warn).toHaveBeenCalledWith(
-      `The dimension invalidDimension doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
-    );
-    expect(console.log).toHaveEmittedEMFWith(
-      expect.objectContaining({ validDimension: 'valid' })
-    );
-    expect(console.log).toHaveEmittedEMFWith(
-      expect.not.objectContaining({ invalidDimension: value })
-    );
-  });
+    {
+      value: 'valid-value',
+      name: '',
+    },
+  ])(
+    'skips invalid dimension values in addDimensions ($name)',
+    ({ value, name }) => {
+      // Prepare
+      const metrics = new Metrics({
+        singleMetric: true,
+        namespace: DEFAULT_NAMESPACE,
+      });
+
+      // Act & Assess
+      metrics.addDimensions({
+        validDimension: 'valid',
+        [name as string]: value as string,
+      });
+      metrics.addMetric('test', MetricUnit.Count, 1);
+      metrics.publishStoredMetrics();
+
+      expect(console.warn).toHaveBeenCalledWith(
+        `The dimension ${name} doesn't meet the requirements and won't be added. Ensure the dimension name and value are non empty strings`
+      );
+      expect(console.log).toHaveEmittedEMFWith(
+        expect.objectContaining({ validDimension: 'valid' })
+      );
+      expect(console.log).toHaveEmittedEMFWith(
+        expect.not.objectContaining({ invalidDimension: value })
+      );
+    }
+  );
 
   it('warns when addDimensions overwrites existing dimensions', () => {
     // Prepare
