Skip to content

Commit 21aff1f

Browse files
hjgracahjgraca
authored
Merge pull request #499 from hjgraca/metrics-decorator-exception
fix: Metrics throws exception when decorating non handler methods
2 parents 654de19 + 95d300b commit 21aff1f

File tree

5 files changed

+99
-24
lines changed

5 files changed

+99
-24
lines changed

‎libraries/src/AWS.Lambda.Powertools.Metrics/IMetrics.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ namespace AWS.Lambda.Powertools.Metrics;
2323
/// Implements the <see cref="System.IDisposable" />
2424
/// </summary>
2525
/// <seealso cref="System.IDisposable" />
26-
public interface IMetrics : IDisposable
26+
public interface IMetrics
2727
{
2828
/// <summary>
2929
/// Adds metric

‎libraries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs‎

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,13 @@ namespace AWS.Lambda.Powertools.Metrics;
2525
/// Implements the <see cref="IMetrics" />
2626
/// </summary>
2727
/// <seealso cref="IMetrics" />
28-
public class Metrics : IMetrics
28+
public class Metrics : IMetrics, IDisposable
2929
{
3030
/// <summary>
3131
/// The instance
3232
/// </summary>
3333
private static IMetrics _instance;
34-
34+
3535
/// <summary>
3636
/// The context
3737
/// </summary>
@@ -65,15 +65,15 @@ public class Metrics : IMetrics
6565
internal Metrics(IPowertoolsConfigurations powertoolsConfigurations, string nameSpace = null, string service = null,
6666
bool raiseOnEmptyMetrics = false, bool captureColdStartEnabled = false)
6767
{
68-
if (_instance != null) return;
68+
_instance ??= this;
6969

70-
_instance = this;
7170
_powertoolsConfigurations = powertoolsConfigurations;
7271
_raiseOnEmptyMetrics = raiseOnEmptyMetrics;
7372
_captureColdStartEnabled = captureColdStartEnabled;
7473
_context = InitializeContext(nameSpace, service, null);
7574

7675
_powertoolsConfigurations.SetExecutionEnvironment(this);
76+
7777
}
7878

7979
/// <summary>
@@ -91,11 +91,11 @@ void IMetrics.AddMetric(string key, double value, MetricUnit unit, MetricResolut
9191
{
9292
if (string.IsNullOrWhiteSpace(key))
9393
throw new ArgumentNullException(
94-
$"'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");
94+
nameof(key), "'AddMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");
9595

9696
if (value < 0) {
9797
throw new ArgumentException(
98-
"'AddMetric' method requires a valid metrics value. Value must be >= 0.");
98+
"'AddMetric' method requires a valid metrics value. Value must be >= 0.", nameof(value));
9999
}
100100

101101
var metrics = _context.GetMetrics();
@@ -150,8 +150,8 @@ string IMetrics.GetService()
150150
void IMetrics.AddDimension(string key, string value)
151151
{
152152
if (string.IsNullOrWhiteSpace(key))
153-
throw new ArgumentNullException(
154-
$"'AddDimension' method requires a valid dimension key. 'Null' or empty values are not allowed.");
153+
throw new ArgumentNullException(nameof(key),
154+
"'AddDimension' method requires a valid dimension key. 'Null' or empty values are not allowed.");
155155

156156
_context.AddDimension(key, value);
157157
}
@@ -168,28 +168,28 @@ void IMetrics.AddDimension(string key, string value)
168168
void IMetrics.AddMetadata(string key, object value)
169169
{
170170
if (string.IsNullOrWhiteSpace(key))
171-
throw new ArgumentNullException(
172-
$"'AddMetadata' method requires a valid metadata key. 'Null' or empty values are not allowed.");
171+
throw new ArgumentNullException(nameof(key),
172+
"'AddMetadata' method requires a valid metadata key. 'Null' or empty values are not allowed.");
173173

174174
_context.AddMetadata(key, value);
175175
}
176176

177177
/// <summary>
178178
/// Implements interface that sets default dimension list
179179
/// </summary>
180-
/// <param name="defaultDimensions">Default Dimension List</param>
180+
/// <param name="defaultDimension">Default Dimension List</param>
181181
/// <exception cref="System.ArgumentNullException">
182182
/// 'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty
183183
/// values are not allowed.
184184
/// </exception>
185-
void IMetrics.SetDefaultDimensions(Dictionary<string, string> defaultDimensions)
185+
void IMetrics.SetDefaultDimensions(Dictionary<string, string> defaultDimension)
186186
{
187-
foreach (var item in defaultDimensions)
187+
foreach (var item in defaultDimension)
188188
if (string.IsNullOrWhiteSpace(item.Key) || string.IsNullOrWhiteSpace(item.Value))
189-
throw new ArgumentNullException(
190-
$"'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty values are not allowed.");
189+
throw new ArgumentNullException(nameof(item.Key),
190+
"'SetDefaultDimensions' method requires a valid key pair. 'Null' or empty values are not allowed.");
191191

192-
_context.SetDefaultDimensions(DictionaryToList(defaultDimensions));
192+
_context.SetDefaultDimensions(DictionaryToList(defaultDimension));
193193
}
194194

195195
/// <summary>
@@ -258,8 +258,8 @@ void IMetrics.PushSingleMetric(string metricName, double value, MetricUnit unit,
258258
Dictionary<string, string> defaultDimensions, MetricResolution metricResolution)
259259
{
260260
if (string.IsNullOrWhiteSpace(metricName))
261-
throw new ArgumentNullException(
262-
$"'PushSingleMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");
261+
throw new ArgumentNullException(nameof(metricName),
262+
"'PushSingleMetric' method requires a valid metrics key. 'Null' or empty values are not allowed.");
263263

264264
using var context = InitializeContext(nameSpace, service, defaultDimensions);
265265
context.AddMetric(metricName, value, unit, metricResolution);
@@ -272,7 +272,21 @@ void IMetrics.PushSingleMetric(string metricName, double value, MetricUnit unit,
272272
/// </summary>
273273
public void Dispose()
274274
{
275-
_instance.Flush();
275+
Dispose(true);
276+
GC.SuppressFinalize(this);
277+
}
278+
279+
/// <summary>
280+
///
281+
/// </summary>
282+
/// <param name="disposing"></param>
283+
protected virtual void Dispose(bool disposing)
284+
{
285+
// Cleanup
286+
if (disposing)
287+
{
288+
_instance.Flush();
289+
}
276290
}
277291

278292
/// <summary>

‎libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/EMFValidationTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -467,7 +467,7 @@ public void WhenMetricIsNegativeValue_ThrowException()
467467

468468
// Assert
469469
var exception = Assert.Throws<ArgumentException>(act);
470-
Assert.Equal("'AddMetric' method requires a valid metrics value. Value must be >= 0.", exception.Message);
470+
Assert.Equal("'AddMetric' method requires a valid metrics value. Value must be >= 0. (Parameter 'value')", exception.Message);
471471

472472
// RESET
473473
handler.ResetForTest();

‎libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandler.cs‎

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,17 +7,49 @@ namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers;
77
public class ExceptionFunctionHandler
88
{
99
[Metrics(Namespace = "ns", Service = "svc")]
10-
public async Task<string> Handle(string input)
10+
public Task<string> Handle(string input)
1111
{
1212
ThisThrows();
13+
return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture));
14+
}
15+
16+
[Metrics(Namespace = "ns", Service = "svc")]
17+
public Task<string> HandleDecoratorOutsideHandler(string input)
18+
{
19+
MethodDecorated();
20+
21+
Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count);
1322

14-
await Task.Delay(1);
23+
return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture));
24+
}
25+
26+
[Metrics(Namespace = "ns", Service = "svc")]
27+
public Task<string> HandleDecoratorOutsideHandlerException(string input)
28+
{
29+
MethodDecorated();
30+
31+
Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count);
32+
33+
ThisThrowsDecorated();
34+
35+
return Task.FromResult(input.ToUpper(CultureInfo.InvariantCulture));
36+
}
1537

16-
return input.ToUpper(CultureInfo.InvariantCulture);
38+
[Metrics(Namespace = "ns", Service = "svc")]
39+
private void MethodDecorated()
40+
{
41+
Metrics.AddMetric($"Metric Name", 1, MetricUnit.Count);
42+
Metrics.AddMetric($"Metric Name Decorated", 1, MetricUnit.Count);
1743
}
1844

1945
private void ThisThrows()
2046
{
2147
throw new NullReferenceException();
2248
}
49+
50+
[Metrics(Namespace = "ns", Service = "svc")]
51+
private void ThisThrowsDecorated()
52+
{
53+
throw new NullReferenceException();
54+
}
2355
}

‎libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/Handlers/ExceptionFunctionHandlerTests.cs‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,35 @@ public async Task Stack_Trace_Included_When_Decorator_Present()
2020
// Assert
2121
var tracedException = await Assert.ThrowsAsync<NullReferenceException>(Handle);
2222
Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart());
23+
}
24+
25+
[Fact]
26+
public async Task Stack_Trace_Included_When_Decorator_Present_In_Method()
27+
{
28+
// Arrange
29+
Metrics.ResetForTest();
30+
var handler = new ExceptionFunctionHandler();
31+
32+
// Act
33+
Task Handle() => handler.HandleDecoratorOutsideHandlerException("whatever");
34+
35+
// Assert
36+
var tracedException = await Assert.ThrowsAsync<NullReferenceException>(Handle);
37+
Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.__a$_around_ThisThrows", tracedException.StackTrace?.TrimStart());
38+
}
39+
40+
[Fact]
41+
public async Task Decorator_In_Non_Handler_Method_Does_Not_Throw_Exception()
42+
{
43+
// Arrange
44+
Metrics.ResetForTest();
45+
var handler = new ExceptionFunctionHandler();
2346

47+
// Act
48+
Task Handle() => handler.HandleDecoratorOutsideHandler("whatever");
49+
50+
// Assert
51+
var tracedException = await Record.ExceptionAsync(Handle);
52+
Assert.Null(tracedException);
2453
}
2554
}

0 commit comments

Comments
 (0)