Skip to content

Commit 8ff0dd3

Browse files
hjgracahjgraca
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>

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

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+
}

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

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+
}

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

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+
}

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

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

‎libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/AWS.Lambda.Powertools.Tracing.Tests.csproj

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,8 @@
1414

1515
<ItemGroup>
1616
<PackageReference Include="coverlet.collector" Version="6.0.0">
17-
<PrivateAssets>all</PrivateAssets>
18-
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
17+
<PrivateAssets>all</PrivateAssets>
18+
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
1919
</PackageReference>
2020
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.7.0" />
2121
<PackageReference Include="NSubstitute" Version="5.0.0" />

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

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.Tracing.Tests.Handlers;
6+
7+
public class ExceptionFunctionHandler
8+
{
9+
[Tracing(CaptureMode = TracingCaptureMode.ResponseAndError)]
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+
}

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

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.Tracing.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.Tracing.Tests.Handlers.ExceptionFunctionHandler.ThisThrows()", tracedException.StackTrace?.TrimStart());
21+
22+
}
23+
}

‎libraries/tests/AWS.Lambda.Powertools.Tracing.Tests/TracingAttributeTest.cs

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -153,13 +153,13 @@ public void Tracing_WhenTracerDisabled_DisablesTracing()
153153
// Cold Start Execution
154154
handler1.OnEntry(eventArgs);
155155
handler1.OnSuccess(eventArgs, results);
156-
object Act1() => handler1.OnException<object>(eventArgs, exception);
156+
void Act1() => handler1.OnException(eventArgs, exception);
157157
handler1.OnExit(eventArgs);
158158

159159
// Warm Start Execution
160160
handler2.OnEntry(eventArgs);
161161
handler2.OnSuccess(eventArgs, results);
162-
object Act2() => handler2.OnException<object>(eventArgs, exception);
162+
void Act2() => handler2.OnException(eventArgs, exception);
163163
handler2.OnExit(eventArgs);
164164

165165
// Assert
@@ -226,13 +226,13 @@ public void Tracing_WhenOutsideOfLambdaEnv_DisablesTracing()
226226
// Cold Start Execution
227227
handler1.OnEntry(eventArgs);
228228
handler1.OnSuccess(eventArgs, results);
229-
object Act1() => handler1.OnException<object>(eventArgs, exception);
229+
void Act1() => handler1.OnException(eventArgs, exception);
230230
handler1.OnExit(eventArgs);
231231

232232
// Warm Start Execution
233233
handler2.OnEntry(eventArgs);
234234
handler2.OnSuccess(eventArgs, results);
235-
object Act2() => handler2.OnException<object>(eventArgs, exception);
235+
void Act2() => handler2.OnException(eventArgs, exception);
236236
handler2.OnExit(eventArgs);
237237

238238
// Assert
@@ -572,7 +572,7 @@ public void OnException_WhenTracerCaptureErrorEnvironmentVariableIsTrue_Captures
572572
var eventArgs = new AspectEventArgs { Name = methodName };
573573

574574
// Act
575-
void Act() => handler.OnException<object>(eventArgs, exception);
575+
void Act() => handler.OnException(eventArgs, exception);
576576

577577
// Assert
578578
Assert.Throws<Exception>((Action)Act);
@@ -601,7 +601,7 @@ public void OnException_WhenTracerCaptureErrorEnvironmentVariableIsTrueFalse_Doe
601601
var eventArgs = new AspectEventArgs { Name = methodName };
602602

603603
// Act
604-
void Act() => handler.OnException<object>(eventArgs, exception);
604+
void Act() => handler.OnException(eventArgs, exception);
605605

606606
// Assert
607607
Assert.Throws<Exception>((Action)Act);
@@ -630,7 +630,7 @@ public void OnException_WhenTracerCaptureModeIsError_CapturesError()
630630
var eventArgs = new AspectEventArgs { Name = methodName };
631631

632632
// Act
633-
void Act() => handler.OnException<object>(eventArgs, exception);
633+
void Act() => handler.OnException(eventArgs, exception);
634634

635635
// Assert
636636
Assert.Throws<Exception>((Action)Act);
@@ -659,7 +659,7 @@ public void OnException_WhenTracerCaptureModeIsResponseAndError_CapturesError()
659659
var eventArgs = new AspectEventArgs { Name = methodName };
660660

661661
// Act
662-
void Act() => handler.OnException<object>(eventArgs, exception);
662+
void Act() => handler.OnException(eventArgs, exception);
663663

664664
// Assert
665665
Assert.Throws<Exception>((Action)Act);
@@ -689,7 +689,7 @@ public void OnException_WhenTracerCaptureModeIsResponse_DoesNotCaptureError()
689689
var eventArgs = new AspectEventArgs { Name = methodName };
690690

691691
// Act
692-
void Act() => handler.OnException<object>(eventArgs, exception);
692+
void Act() => handler.OnException(eventArgs, exception);
693693

694694
// Assert
695695
Assert.Throws<Exception>((Action)Act);
@@ -718,7 +718,7 @@ public void OnException_WhenTracerCaptureModeIsDisabled_DoesNotCaptureError()
718718
var eventArgs = new AspectEventArgs { Name = methodName };
719719

720720
// Act
721-
void Act() => handler.OnException<object>(eventArgs, exception);
721+
void Act() => handler.OnException(eventArgs, exception);
722722

723723
// Assert
724724
Assert.Throws<Exception>((Action)Act);

0 commit comments

Comments
 (0)