Skip to content

Commit 8ff0dd3

Browse files
authored
Merge pull request #357 from hjgraca/fix-capture-stacktrace
2 parents 902d6a6 + 277ff0d commit 8ff0dd3

File tree

14 files changed

+173
-28
lines changed

14 files changed

+173
-28
lines changed

libraries/src/AWS.Lambda.Powertools.Common/Aspects/IMethodAspectHandler.cs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,11 +38,10 @@ public interface IMethodAspectHandler
3838
/// <summary>
3939
/// Called when [exception].
4040
/// </summary>
41-
/// <typeparam name="T"></typeparam>
4241
/// <param name="eventArgs">The <see cref="AspectEventArgs" /> instance containing the event data.</param>
4342
/// <param name="exception">The exception.</param>
4443
/// <returns>T.</returns>
45-
T OnException<T>(AspectEventArgs eventArgs, Exception exception);
44+
void OnException(AspectEventArgs eventArgs, Exception exception);
4645

4746
/// <summary>
4847
/// Handles the <see cref="E:Exit" /> event.

libraries/src/AWS.Lambda.Powertools.Common/Aspects/MethodAspectAttribute.cs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,8 @@ protected internal sealed override T WrapSync<T>(Func<object[], T> target, objec
6464
}
6565
catch (Exception exception)
6666
{
67-
return AspectHandler.OnException<T>(eventArgs, exception);
67+
AspectHandler.OnException(eventArgs, exception);
68+
throw;
6869
}
6970
finally
7071
{
@@ -92,7 +93,8 @@ protected internal sealed override async Task<T> WrapAsync<T>(Func<object[], Tas
9293
}
9394
catch (Exception exception)
9495
{
95-
return AspectHandler.OnException<T>(eventArgs, exception);
96+
AspectHandler.OnException(eventArgs, exception);
97+
throw;
9698
}
9799
finally
98100
{

libraries/src/AWS.Lambda.Powertools.Logging/Internal/LoggingAspectHandler.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
using System;
1717
using System.IO;
1818
using System.Linq;
19+
using System.Runtime.ExceptionServices;
1920
using System.Text.Json;
2021
using System.Text.Json.Serialization;
2122
using AWS.Lambda.Powertools.Common;
@@ -203,16 +204,16 @@ public void OnSuccess(AspectEventArgs eventArgs, object result)
203204
/// <summary>
204205
/// Called when [exception].
205206
/// </summary>
206-
/// <typeparam name="T"></typeparam>
207207
/// <param name="eventArgs">
208208
/// The <see cref="T:AWS.Lambda.Powertools.Aspects.AspectEventArgs" /> instance containing the
209209
/// event data.
210210
/// </param>
211211
/// <param name="exception">The exception.</param>
212-
/// <returns>T.</returns>
213-
public T OnException<T>(AspectEventArgs eventArgs, Exception exception)
212+
public void OnException(AspectEventArgs eventArgs, Exception exception)
214213
{
215-
throw exception;
214+
// The purpose of ExceptionDispatchInfo.Capture is to capture a potentially mutating exception's StackTrace at a point in time:
215+
// https://learn.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions#capture-exceptions-to-rethrow-later
216+
ExceptionDispatchInfo.Capture(exception).Throw();
216217
}
217218

218219
/// <summary>

libraries/src/AWS.Lambda.Powertools.Metrics/Internal/MetricsAspectHandler.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515

1616
using System;
1717
using System.Collections.Generic;
18+
using System.Runtime.ExceptionServices;
1819
using AWS.Lambda.Powertools.Common;
1920

2021
namespace AWS.Lambda.Powertools.Metrics;
@@ -122,14 +123,14 @@ public void OnSuccess(AspectEventArgs eventArgs, object result)
122123
/// <summary>
123124
/// OnException runs when an unhandled exception occurs inside the method marked with the Metrics Attribute
124125
/// </summary>
125-
/// <typeparam name="T">Type of Exception expected</typeparam>
126126
/// <param name="eventArgs">Aspect Arguments</param>
127127
/// <param name="exception">Exception thrown by the method marked with Metrics Attribute</param>
128-
/// <returns>Type of the Exception expected</returns>
129128
/// <exception cref="Exception">Generic unhandled exception</exception>
130-
public T OnException<T>(AspectEventArgs eventArgs, Exception exception)
129+
public void OnException(AspectEventArgs eventArgs, Exception exception)
131130
{
132-
throw exception;
131+
// The purpose of ExceptionDispatchInfo.Capture is to capture a potentially mutating exception's StackTrace at a point in time:
132+
// https://learn.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions#capture-exceptions-to-rethrow-later
133+
ExceptionDispatchInfo.Capture(exception).Throw();
133134
}
134135

135136
/// <summary>

libraries/src/AWS.Lambda.Powertools.Tracing/Internal/TracingAspectHandler.cs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
*/
1515

1616
using System;
17+
using System.Runtime.ExceptionServices;
1718
using System.Text;
1819
using AWS.Lambda.Powertools.Common;
1920

@@ -152,14 +153,12 @@ public void OnSuccess(AspectEventArgs eventArgs, object result)
152153
/// <summary>
153154
/// Called when [exception].
154155
/// </summary>
155-
/// <typeparam name="T"></typeparam>
156156
/// <param name="eventArgs">
157157
/// The <see cref="T:AWS.Lambda.Powertools.Aspects.AspectEventArgs" /> instance containing the
158158
/// event data.
159159
/// </param>
160160
/// <param name="exception">The exception.</param>
161-
/// <returns>T.</returns>
162-
public T OnException<T>(AspectEventArgs eventArgs, Exception exception)
161+
public void OnException(AspectEventArgs eventArgs, Exception exception)
163162
{
164163
if (CaptureError())
165164
{
@@ -187,7 +186,9 @@ public T OnException<T>(AspectEventArgs eventArgs, Exception exception)
187186
);
188187
}
189188

190-
throw exception;
189+
// The purpose of ExceptionDispatchInfo.Capture is to capture a potentially mutating exception's StackTrace at a point in time:
190+
// https://learn.microsoft.com/en-us/dotnet/standard/exceptions/best-practices-for-exceptions#capture-exceptions-to-rethrow-later
191+
ExceptionDispatchInfo.Capture(exception).Throw();
191192
}
192193

193194
/// <summary>
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using System;
2+
using System.Globalization;
3+
using System.Threading.Tasks;
4+
5+
namespace AWS.Lambda.Powertools.Logging.Tests.Handlers;
6+
7+
public class ExceptionFunctionHandler
8+
{
9+
[Logging]
10+
public async Task<string> Handle(string input)
11+
{
12+
ThisThrows();
13+
14+
await Task.Delay(1);
15+
16+
return input.ToUpper(CultureInfo.InvariantCulture);
17+
}
18+
19+
private void ThisThrows()
20+
{
21+
throw new NullReferenceException();
22+
}
23+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using Xunit;
4+
5+
namespace AWS.Lambda.Powertools.Logging.Tests.Handlers;
6+
7+
public sealed class ExceptionFunctionHandlerTests
8+
{
9+
[Fact]
10+
public async Task Stack_Trace_Included_When_Decorator_Present()
11+
{
12+
// Arrange
13+
var handler = new ExceptionFunctionHandler();
14+
15+
// Act
16+
Task Handle() => handler.Handle("whatever");
17+
18+
// Assert
19+
var tracedException = await Assert.ThrowsAsync<NullReferenceException>(Handle);
20+
Assert.StartsWith("at AWS.Lambda.Powertools.Logging.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart());
21+
22+
}
23+
}
Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
using System;
2+
using System.Globalization;
3+
using System.Threading.Tasks;
4+
5+
namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers;
6+
7+
public class ExceptionFunctionHandler
8+
{
9+
[Metrics(Namespace = "ns", Service = "svc")]
10+
public async Task<string> Handle(string input)
11+
{
12+
ThisThrows();
13+
14+
await Task.Delay(1);
15+
16+
return input.ToUpper(CultureInfo.InvariantCulture);
17+
}
18+
19+
private void ThisThrows()
20+
{
21+
throw new NullReferenceException();
22+
}
23+
}
Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
using System;
2+
using System.Threading.Tasks;
3+
using Xunit;
4+
5+
namespace AWS.Lambda.Powertools.Metrics.Tests.Handlers;
6+
7+
[Collection("Sequential")]
8+
public sealed class ExceptionFunctionHandlerTests
9+
{
10+
[Fact]
11+
public async Task Stack_Trace_Included_When_Decorator_Present()
12+
{
13+
// Arrange
14+
Metrics.ResetForTest();
15+
var handler = new ExceptionFunctionHandler();
16+
17+
// Act
18+
Task Handle() => handler.Handle("whatever");
19+
20+
// Assert
21+
var tracedException = await Assert.ThrowsAsync<NullReferenceException>(Handle);
22+
Assert.StartsWith("at AWS.Lambda.Powertools.Metrics.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart());
23+
24+
}
25+
}

libraries/tests/AWS.Lambda.Powertools.Metrics.Tests/MetricsTests.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ public class MetricsTests
1111
public void Metrics_Set_Execution_Environment_Context()
1212
{
1313
// Arrange
14+
Metrics.ResetForTest();
1415
var assemblyName = "AWS.Lambda.Powertools.Metrics";
1516
var assemblyVersion = "1.0.0";
1617

0 commit comments

Comments
 (0)