Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Custom exception JSON converter #152

Merged

Conversation

amirkaws
Copy link
Contributor

Issue number: #151

Summary

This PR adds a custom JSON converter to serialize Exception

Changes

Please provide a summary of what's being changed
A custom exception JSON converter has been added to the JsonSerialiser for Logger

User experience

Please share what the user experience looks like before and after this change

Checklist

Please leave checklist items unchecked if they do not apply to your change.

Is this a breaking change? No

RFC issue number:

Checklist:

  • Migration process documented
  • Implement warnings (if it can live side by side)

Acknowledgment

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

@amirkaws amirkaws added bug Unexpected, reproducible and unintended software behaviour area/logging Core logging utility feature New features or minor changes pending-release Fix or implementation already in dev waiting to be released labels Aug 17, 2022
@auto-assign auto-assign bot requested a review from sliedig August 17, 2022 14:42
@codecov-commenter
Copy link

codecov-commenter commented Aug 17, 2022

Codecov Report

Merging #152 (2140040) into develop (e81d86a) will increase coverage by 1.28%.
The diff coverage is 80.17%.

@@             Coverage Diff             @@
##           develop     #152      +/-   ##
===========================================
+ Coverage    55.01%   56.29%   +1.28%     
===========================================
  Files           37       41       +4     
  Lines         1685     1778      +93     
===========================================
+ Hits           927     1001      +74     
- Misses         758      777      +19     
Flag Coverage Δ
unittests 56.29% <80.17%> (+1.28%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...gging/Internal/Converters/MemoryStreamConverter.cs 60.00% <60.00%> (ø)
...ging/Internal/Converters/ConstantClassConverter.cs 64.28% <64.28%> (ø)
.../AWS.Lambda.Powertools.Logging/LoggerExtensions.cs 36.64% <66.66%> (+2.53%) ⬆️
....Logging/Internal/Converters/ByteArrayConverter.cs 68.75% <68.75%> (ø)
....Logging/Internal/Converters/ExceptionConverter.cs 72.97% <72.97%> (ø)
...owertools.Logging/Internal/LoggingAspectHandler.cs 73.29% <100.00%> (+2.15%) ⬆️
...da.Powertools.Logging/Internal/PowertoolsLogger.cs 90.36% <100.00%> (+1.05%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@jakejscott
Copy link

Can we include the stack trace too?

@amirkaws
Copy link
Contributor Author

amirkaws commented Aug 19, 2022

It already logs all public properties, including StackTrace. I've also tested the logger with a custom exception contains Type & intPtr properties.

CustomExceptionLog

@amirkaws
Copy link
Contributor Author

To support Lambda input event serialization, following custom converters from Amazon.Lambda.Serialization.SystemTextJson have been added:

@jeastham1993 jeastham1993 removed the request for review from sliedig September 9, 2022 15:49
@sliedig sliedig merged commit 6a2e23a into aws-powertools:develop Sep 12, 2022
@amirkaws amirkaws deleted the amirkaws-custom-exception-json-converter branch September 12, 2022 11:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/logging Core logging utility bug Unexpected, reproducible and unintended software behaviour feature New features or minor changes pending-release Fix or implementation already in dev waiting to be released
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants