From e0af7f5e948076aab560218f82b6f4bb59dfa742 Mon Sep 17 00:00:00 2001 From: svozza Date: Sun, 26 Apr 2026 19:57:59 +0100 Subject: [PATCH 1/2] fix(metrics): clear state in publishStoredMetrics even when throwOnEmptyMetrics throws When serializeMetrics() threw due to throwOnEmptyMetrics, the cleanup calls (clearMetrics, clearDimensions, clearMetadata) were never reached, causing dimensions and metadata to leak into subsequent publishes. Wrap the serialize/emit block in try/finally to guarantee cleanup. Also add tests for the disabled code path, removing the v8 ignore comment. Closes #5207 --- packages/metrics/src/Metrics.ts | 17 +++--- .../tests/unit/creatingMetrics.test.ts | 55 +++++++++++++++++++ 2 files changed, 64 insertions(+), 8 deletions(-) diff --git a/packages/metrics/src/Metrics.ts b/packages/metrics/src/Metrics.ts index 7d96fa470a..94d615c108 100644 --- a/packages/metrics/src/Metrics.ts +++ b/packages/metrics/src/Metrics.ts @@ -609,15 +609,16 @@ class Metrics extends Utility implements MetricsInterface { ); } - /* v8 ignore else -- @preserve */ - if (!this.disabled) { - const emfOutput = this.serializeMetrics(); - hasMetrics && this.console.log(JSON.stringify(emfOutput)); + try { + if (!this.disabled) { + const emfOutput = this.serializeMetrics(); + hasMetrics && this.console.log(JSON.stringify(emfOutput)); + } + } finally { + this.clearMetrics(); + this.clearDimensions(); + this.clearMetadata(); } - - this.clearMetrics(); - this.clearDimensions(); - this.clearMetadata(); return this; } diff --git a/packages/metrics/tests/unit/creatingMetrics.test.ts b/packages/metrics/tests/unit/creatingMetrics.test.ts index 82123a3a48..ec41739abb 100644 --- a/packages/metrics/tests/unit/creatingMetrics.test.ts +++ b/packages/metrics/tests/unit/creatingMetrics.test.ts @@ -209,6 +209,61 @@ describe('Creating metrics', () => { ); }); + it('clears dimensions and metadata even when throwOnEmptyMetrics throws', () => { + // Prepare + const metrics = new Metrics({ + singleMetric: false, + namespace: DEFAULT_NAMESPACE, + }); + metrics.setThrowOnEmptyMetrics(true); + metrics.addDimension('request_id', 'abc-123'); + metrics.addMetadata('trace_id', 'xyz-789'); + + // Act - first publish throws because no metrics were added + expect(() => metrics.publishStoredMetrics()).toThrowError( + 'The number of metrics recorded must be higher than zero' + ); + + // Publish again with a metric - dimensions and metadata should be clean + metrics.setThrowOnEmptyMetrics(false); + metrics.addMetric('test', MetricUnit.Count, 1); + metrics.publishStoredMetrics(); + + // Assess - leaked state should not appear in the output + expect(console.log).toHaveEmittedNthEMFWith( + 1, + expect.not.objectContaining({ + request_id: 'abc-123', + }) + ); + expect(console.log).toHaveEmittedNthEMFWith( + 1, + expect.not.objectContaining({ + trace_id: 'xyz-789', + }) + ); + }); + + it('does not emit metrics when disabled but still clears state', () => { + // Prepare + process.env.POWERTOOLS_DEV = undefined; + process.env.POWERTOOLS_METRICS_DISABLED = 'true'; + const metrics = new Metrics({ + singleMetric: false, + namespace: DEFAULT_NAMESPACE, + }); + metrics.addMetric('test', MetricUnit.Count, 1); + metrics.addDimension('env', 'prod'); + metrics.addMetadata('trace', '123'); + + // Act + metrics.publishStoredMetrics(); + + // Assess + expect(console.log).not.toHaveBeenCalled(); + expect(metrics.hasStoredMetrics()).toBe(false); + }); + it('flushes the buffer automatically when the buffer is full', () => { // Prepare const metrics = new Metrics({ From 18ed7a07b9de174090c49b9f191b5e9e71e5bfbb Mon Sep 17 00:00:00 2001 From: svozza Date: Sun, 26 Apr 2026 20:47:53 +0100 Subject: [PATCH 2/2] refactor(metrics): remove commentary comments from tests --- packages/metrics/tests/unit/creatingMetrics.test.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/packages/metrics/tests/unit/creatingMetrics.test.ts b/packages/metrics/tests/unit/creatingMetrics.test.ts index ec41739abb..4e61fe0233 100644 --- a/packages/metrics/tests/unit/creatingMetrics.test.ts +++ b/packages/metrics/tests/unit/creatingMetrics.test.ts @@ -219,17 +219,15 @@ describe('Creating metrics', () => { metrics.addDimension('request_id', 'abc-123'); metrics.addMetadata('trace_id', 'xyz-789'); - // Act - first publish throws because no metrics were added + // Act expect(() => metrics.publishStoredMetrics()).toThrowError( 'The number of metrics recorded must be higher than zero' ); - - // Publish again with a metric - dimensions and metadata should be clean metrics.setThrowOnEmptyMetrics(false); metrics.addMetric('test', MetricUnit.Count, 1); metrics.publishStoredMetrics(); - // Assess - leaked state should not appear in the output + // Assess expect(console.log).toHaveEmittedNthEMFWith( 1, expect.not.objectContaining({