fix: Handle Exceptions and Prevent application from crashing when using Logger

hjgraca commented 8 months ago

Please provide the issue number

Issue number: #514

Summary

On exceptions within the Logging framework code, currently, it throws an exception and brings down the whole application.
For example below is a code that has a mismatch count of parameters passed to the log message - Missing Count property value being passed in.

Logger.LogInformation(extraKeys, "Retrieved data for city {cityName} with count {Count}", new[] { cityName });

It throws an exception and causes the Lambda Handler endpoint to crash and return a 500 Internal Server Error

System.IndexOutOfRangeException: Index was outside the bounds of the array.
   at Microsoft.Extensions.Logging.LogValuesFormatter.GetValue(Object[] values, Int32 index)
   at Microsoft.Extensions.Logging.FormattedLogValues.get_Item(Int32 index)
   at Microsoft.Extensions.Logging.FormattedLogValues.GetEnumerator()+MoveNext(

Changes

Add exception handling in Log extension method.

try
{
    Log(logger, logLevel, extraKeys, 0, null, message, args);
}
catch (Exception e)
{
    logger.Log(LogLevel.Error, 0, e, "Powertools internal error");
}

Add Tests

User experience

The client should not see any exception in case Logger fails internally. It will be logged as error to get visibility

Checklist

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

[x] Meets tenets criteria
[x] I have performed a self-review of this change
[x] Changes have been tested
[x] Changes are documented
[x] PR title follows conventional commit semantics

Is this a breaking change?

**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.

sonarcloud[bot] commented 8 months ago

Quality Gate passed

Kudos, no new issues were introduced!

0 New issues
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

codecov-commenter commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (7308cc6) 72.23% compared to head (5159cd8) 73.32%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## develop #538 +/- ## =========================================== + Coverage 72.23% 73.32% +1.08% =========================================== Files 101 101 Lines 4106 4112 +6 Branches 418 418 =========================================== + Hits 2966 3015 +49 + Misses 1026 977 -49 - Partials 114 120 +6 ``` | [Flag](https://app.codecov.io/gh/aws-powertools/powertools-lambda-dotnet/pull/538/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/aws-powertools/powertools-lambda-dotnet/pull/538/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `73.32% <100.00%> (+1.08%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

aws-powertools / powertools-lambda-dotnet