Skip to content

Commit

Permalink
Populate required field Message with n/a if it is empty (#2857)
Browse files Browse the repository at this point in the history
* Populate required field Message with n/a if it is empty

* Move using Microsoft.Application.Insights.DataContracts to be within namespace

* Address PR comments by fixing test to use message, and using a utility method to check for null/whitespace
  • Loading branch information
NeilMountford authored Mar 27, 2024
1 parent bd8b0c2 commit e2d1d0b
Show file tree
Hide file tree
Showing 4 changed files with 45 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,19 @@ public void DoesNotTrimShortExceptionMessages()
Assert.AreEqual(5, expDetails.message.Length);
}

[TestMethod]
[DataRow(null)]
[DataRow("")]
[DataRow(" ")]
public void ProvidesDefaultMessage(string message)
{
var exp = new ExceptionWithMessageOverride(message);

ExceptionDetails expDetails = ExceptionConverter.ConvertToExceptionDetails(exp, null);

Assert.AreEqual("n/a", expDetails.message);
}

[MethodImplAttribute(MethodImplOptions.NoInlining)]
private Exception CreateException(int numberOfStackpoints)
{
Expand Down Expand Up @@ -212,5 +225,20 @@ private void FailedFunction(int numberOfStackpoints)

throw new AggregateException("exception message");
}

/// <summary>
/// Overrides the Message property of the base exception, so that null messages are
/// returned as null, rather than a default, e.g. "Exception of type 'System.Exception' was thrown".
/// </summary>
private class ExceptionWithMessageOverride : Exception
{
public ExceptionWithMessageOverride(string message)
: base(message)
{
Message = message;
}

public override string Message { get; }
}
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
namespace Microsoft.ApplicationInsights.Extensibility.Implementation.External
{
using System;
using Microsoft.ApplicationInsights.DataContracts;

/// <summary>
/// Additional implementation for ExceptionDetails.
Expand All @@ -21,7 +22,7 @@ internal static ExceptionDetails CreateWithoutStackInfo(Exception exception, Exc
{
id = exception.GetHashCode(),
typeName = exception.GetType().FullName,
message = exception.Message,
message = Utils.PopulateRequiredNonWhitespaceStringValue(exception.Message, "message", typeof(ExceptionTelemetry).FullName),
};

if (parentExceptionDetails != null)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,20 @@ public static string PopulateRequiredStringValue(string value, string parameterN
return value;
}

/// <summary>
/// Validates the string and if null or empty populates it with '$parameterName is a required field for $telemetryType' value.
/// </summary>
public static string PopulateRequiredNonWhitespaceStringValue(string value, string parameterName, string telemetryType)
{
if (value.IsNullOrWhiteSpace())
{
CoreEventSource.Log.PopulateRequiredStringWithValue(parameterName, telemetryType);
return "n/a";
}

return value;
}

/// <summary>
/// Returns default Timespan value if not a valid Timespan.
/// </summary>
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# Changelog

## VNext
- [Populate required field Message with "n/a" if it is empty](https://github.com/microsoft/ApplicationInsights-dotnet/issues/1066)

## Version 2.22.0
- no changes since beta.
Expand Down

0 comments on commit e2d1d0b

Please sign in to comment.