Merge pull request 7956 from hotfix/v4.4.8 into master

51Degrees · Dec 5, 2022 · 22db525 · 22db525
2 parents c4f920b + 10b06f9
commit 22db525
Show file tree

Hide file tree

Showing 16 changed files with 108 additions and 108 deletions.
diff --git a/51D.md b/51D.md
diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FiftyOne.Pipeline.CloudRequestEngine.csproj b/FiftyOne.Pipeline.CloudRequestEngine/FiftyOne.Pipeline.CloudRequestEngine.csproj
@@ -36,10 +36,6 @@
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="1.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
   </ItemGroup>
 
   <ItemGroup>

diff --git a/FiftyOne.Pipeline.Core/FiftyOne.Pipeline.Core.csproj b/FiftyOne.Pipeline.Core/FiftyOne.Pipeline.Core.csproj
@@ -39,10 +39,6 @@
     </PackageReference>
     <PackageReference Include="Microsoft.Extensions.Configuration.Binder" Version="2.1.10" />
     <PackageReference Include="Microsoft.Extensions.Logging" Version="2.1.1" />
-    <PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="1.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
   </ItemGroup>
 
   <ItemGroup>

diff --git a/...nts/FiftyOne.Pipeline.JavaScriptBuilderElement/FiftyOne.Pipeline.JavaScriptBuilder.csproj b/...nts/FiftyOne.Pipeline.JavaScriptBuilderElement/FiftyOne.Pipeline.JavaScriptBuilder.csproj
@@ -49,10 +49,6 @@
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="1.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
     <PackageReference Include="NUglify" Version="1.5.12" />
     <PackageReference Include="Stubble.Core" Version="1.7.2" />
     <PackageReference Include="System.CodeDom" Version="4.5.0" />

diff --git a/...peline.Elements/FiftyOne.Pipeline.JsonBuilderElement/FiftyOne.Pipeline.JsonBuilder.csproj b/...peline.Elements/FiftyOne.Pipeline.JsonBuilderElement/FiftyOne.Pipeline.JsonBuilder.csproj
@@ -36,10 +36,6 @@
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="1.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
     <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
   </ItemGroup>
 

diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FiftyOne.Pipeline.Engines.FiftyOne.csproj b/FiftyOne.Pipeline.Engines.FiftyOne/FiftyOne.Pipeline.Engines.FiftyOne.csproj
@@ -49,10 +49,6 @@
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="1.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
   </ItemGroup>
 
   <ItemGroup>

diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/SetHeadersElement.cs b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/SetHeadersElement.cs
@@ -153,32 +153,44 @@ private Dictionary<string, string> BuildResponseHeaderDictionary(
             {
                 // Get the value for this property.
                 var elementData = data.Get(property.Value.PropertyMetaData.Element.ElementDataKey);
-                var propertyValue = elementData[property.Key];
-                // Extract the string value.
-                var headerValue = GetHeaderValue(propertyValue);
-
-                // If value is not blank, null or 'unknown' then
-                // add it to the complete value for the associated
-                // header.
-                if(string.IsNullOrEmpty(headerValue) == false &&
-                    headerValue.Equals("Unknown", StringComparison.OrdinalIgnoreCase) == false)
+                object propertyValue = null;
+                try
                 {
-                    HashSet<string> values;
-                    if(result.TryGetValue(property.Value.ResponseHeaderName, out values) == false)
-                    {
-                        // No entry for this header name so create it.
-                        values = new HashSet<string>();
-                        result.Add(property.Value.ResponseHeaderName, values);
-                    }
-                    // Get the individual entries from the comma-separated value for this
-                    // property. If each entry is not already in the list of values for this 
-                    // header then add it.
-                    foreach (var value in headerValue.Split(',')
-                        .Select(v => v.Trim()))
+                    propertyValue = elementData[property.Key];
+                }
+                catch (PropertyMissingException e)
+                {
+                    Logger.LogWarning($"Property '${property.Key}' was missing. " +
+                        $"Not adding SetHeader value. Error was: ${e.Message}");
+                }
+                if (propertyValue != null)
+                {
+                    // Extract the string value.
+                    var headerValue = GetHeaderValue(propertyValue);
+
+                    // If value is not blank, null or 'unknown' then
+                    // add it to the complete value for the associated
+                    // header.
+                    if (string.IsNullOrEmpty(headerValue) == false &&
+                        headerValue.Equals("Unknown", StringComparison.OrdinalIgnoreCase) == false)
                     {
-                        if(values.Contains(value) == false)
+                        HashSet<string> values;
+                        if (result.TryGetValue(property.Value.ResponseHeaderName, out values) == false)
+                        {
+                            // No entry for this header name so create it.
+                            values = new HashSet<string>();
+                            result.Add(property.Value.ResponseHeaderName, values);
+                        }
+                        // Get the individual entries from the comma-separated value for this
+                        // property. If each entry is not already in the list of values for this 
+                        // header then add it.
+                        foreach (var value in headerValue.Split(',')
+                            .Select(v => v.Trim()))
                         {
-                            values.Add(value);
+                            if (values.Contains(value) == false)
+                            {
+                                values.Add(value);
+                            }
                         }
                     }
                 }

diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBase.cs b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageBase.cs
@@ -916,15 +916,11 @@ protected void TrySendData()
                             BuildAndSendXml();
                         }).ContinueWith(t =>
                         {
-                            // If an uncaught error has occurred then 
-                            // cancel further usage sharing and log
-                            // an error.
                             if(t.Exception != null)
                             {
-                                IsCanceled = true;
                                 Logger.LogError(
                                     t.Exception,
-                                    Messages.MessageShareUsageCancelled);
+                                    Messages.MessageShareUsageUnexpectedFailure);
                             }
                         }, TaskScheduler.Default);
                     }

diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageElement.cs b/FiftyOne.Pipeline.Engines.FiftyOne/FlowElements/ShareUsageElement.cs
@@ -25,11 +25,13 @@
 using Microsoft.Extensions.Logging;
 using System;
 using System.Collections.Generic;
+using System.Globalization;
 using System.IO;
 using System.IO.Compression;
 using System.Net;
 using System.Net.Http;
 using System.Runtime.CompilerServices;
+using System.Threading;
 using System.Xml;
 
 [assembly: InternalsVisibleTo("FiftyOne.Pipeline.Engines.FiftyOne.Tests")]
@@ -44,6 +46,9 @@ namespace FiftyOne.Pipeline.Engines.FiftyOne.FlowElements
     /// </summary>
     public class ShareUsageElement : ShareUsageBase
     {
+        private long _successCount = 0;
+        private long _failCount = 0;
+
         /// <inheritdoc/>
         internal ShareUsageElement(
             ILogger<ShareUsageBase> logger,
@@ -118,11 +123,40 @@ protected override void BuildAndSendXml()
                     content.Headers.Add("content-type", "text/xml");
 
                     var res = HttpClient.PostAsync(ShareUsageUri, content).Result;
-                    if (res.StatusCode != HttpStatusCode.OK)
+                    if (res.StatusCode == HttpStatusCode.OK)
+                    {
+                        Interlocked.Increment(ref _successCount);
+                    } 
+                    else
                     {
-                        throw new HttpRequestException(
-                            $"HTTP response was {res.StatusCode}: " +
-                            $"{res.Content.ToString()}.");
+                        Interlocked.Increment(ref _failCount);
+                        string response = "";
+                        try
+                        {
+                            response = res.Content.ReadAsStringAsync().Result;
+                        }
+                        // Ignore any failures to read the response content, we're only logging it.
+#pragma warning disable CA1031 // Do not catch general exception types
+                        catch (Exception) { }
+#pragma warning restore CA1031 // Do not catch general exception types
+
+                        // If failure percentage is > 1% then log this as a warning.
+                        // Otherwise, log at information level.
+                        var logLevel = LogLevel.Information;
+                        var percentageFailure = _failCount / (_successCount + _failCount);
+                        if (percentageFailure > 0.01)
+                        {
+                            logLevel = LogLevel.Warning;
+                        }
+
+                        var message = string.Format(CultureInfo.CurrentCulture,
+                            Messages.MessageShareUsageFailedToSend,
+                            percentageFailure,
+                            ShareUsageUri,
+                            (int)res.StatusCode,
+                            res.StatusCode,
+                            response);
+                        Logger.Log(logLevel, message);
                     }
                 }
             }

diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/Messages.Designer.cs b/FiftyOne.Pipeline.Engines.FiftyOne/Messages.Designer.cs
diff --git a/FiftyOne.Pipeline.Engines.FiftyOne/Messages.resx b/FiftyOne.Pipeline.Engines.FiftyOne/Messages.resx
@@ -138,12 +138,12 @@
   <data name="MessageFailSequenceNumberRetreive" xml:space="preserve">
     <value>Failed to retrieve sequence number</value>
   </data>
-  <data name="MessageShareUsageCancelled" xml:space="preserve">
-    <value>Usage sharing was canceled due to an error</value>
-  </data>
   <data name="MessageShareUsageFailedToAddData" xml:space="preserve">
     <value>Share usage was canceled after failing to add data to the collection. This may mean that the max collection size is too low for the amount of traffic / min devices to send, or that the 'send' thread has stopped taking data from the collection</value>
   </data>
+  <data name="MessageShareUsageFailedToSend" xml:space="preserve">
+    <value>Failure sending usage data. {0:P2} of usage sharing messages are failing. Occasional failures are expected, but large numbers of failures may indicate a network problem connecting to the usage sharing API @ {1}. HTTP response code: {2} - {3}. Response content: {4}.</value>
+  </data>
   <data name="MessageShareUsageInvalidConfig" xml:space="preserve">
     <value>Configuration for '{0}' is invalid.{1}</value>
   </data>
@@ -153,4 +153,7 @@
   <data name="MessageShareUsageTooManyPipelines" xml:space="preserve">
     <value>Share usage element registered to {0} Pipelines. Unable to populate flow element information</value>
   </data>
+  <data name="MessageShareUsageUnexpectedFailure" xml:space="preserve">
+    <value>Unexpected failure while sharing usage data. Please see exception message for details.</value>
+  </data>
 </root>
diff --git a/FiftyOne.Pipeline.Engines/FiftyOne.Pipeline.Engines.csproj b/FiftyOne.Pipeline.Engines/FiftyOne.Pipeline.Engines.csproj
@@ -43,10 +43,6 @@
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="1.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
     <PackageReference Include="Newtonsoft.Json" Version="13.0.1" />
   </ItemGroup>
 

diff --git a/FiftyOne.Pipeline.Web.Shared/FiftyOne.Pipeline.Web.Shared.csproj b/FiftyOne.Pipeline.Web.Shared/FiftyOne.Pipeline.Web.Shared.csproj
@@ -34,10 +34,6 @@
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
     </PackageReference>
-    <PackageReference Include="Microsoft.SourceLink.AzureRepos.Git" Version="1.0.0">
-      <PrivateAssets>all</PrivateAssets>
-      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
-    </PackageReference>
   </ItemGroup>
 
   <ItemGroup>

diff --git a/Tests/FiftyOne.Pipeline.Engines.FiftyOne.Tests/FlowElements/ShareUsageElementTests.cs b/Tests/FiftyOne.Pipeline.Engines.FiftyOne.Tests/FlowElements/ShareUsageElementTests.cs
@@ -36,6 +36,8 @@
 using System.Xml;
 using FiftyOne.Common.TestHelpers;
 using FiftyOne.Pipeline.Engines.TestHelpers;
+using System;
+using System.Linq;
 
 namespace FiftyOne.Pipeline.Engines.FiftyOne.Tests.FlowElements
 {
@@ -48,7 +50,7 @@ public class ShareUsageElementTests
         private ShareUsageElement _shareUsageElement;
 
         // Mocks and dependencies
-        private Mock<ILogger<ShareUsageElement>> _logger;
+        private TestLogger<ShareUsageElement> _logger;
         private Mock<MockHttpMessageHandler> _httpHandler;
         private Mock<IPipeline> _pipeline;
         private Mock<ITracker> _tracker;
@@ -63,7 +65,7 @@ public class ShareUsageElementTests
         [TestInitialize]
         public void Init()
         {
-            _logger = new Mock<ILogger<ShareUsageElement>>();
+            _logger = new TestLogger<ShareUsageElement>();
 
             // Create the HttpClient using the mock handler
             _httpHandler = new Mock<MockHttpMessageHandler>() { CallBase = true };
@@ -105,7 +107,7 @@ private void CreateShareUsage(double sharePercentage,
             _sequenceElement = new SequenceElement(new Mock<ILogger<SequenceElement>>().Object);
             _sequenceElement.AddPipeline(_pipeline.Object);
             _shareUsageElement = new ShareUsageElement(
-                _logger.Object,
+                _logger,
                 _httpClient,
                 sharePercentage,
                 minimumEntriesPerMessage,
@@ -360,11 +362,10 @@ public void ShareUsageElement_SendOnCleanup()
         }
 
         /// <summary>
-        /// Test that the ShareUsageElement stops cleanly if there is an HTTP
-        /// error when sending data to the remote service.
+        /// Test that the ShareUsageElement will log a warning if errors occur when sending
         /// </summary>
         [TestMethod]
-        public void ShareUsageElement_CancelOnServerError()
+        public void ShareUsageElement_LogErrors()
         {
             // Arrange
             _httpHandler.Setup(h => h.Send(It.IsAny<HttpRequestMessage>()))
@@ -385,9 +386,10 @@ public void ShareUsageElement_CancelOnServerError()
             _shareUsageElement.SendDataTask.Wait();
 
             // Assert
-            // Check that no HTTP messages were sent.
-            _httpHandler.Verify(h => h.Send(It.IsAny<HttpRequestMessage>()), Times.Once);
-            Assert.IsTrue(_shareUsageElement.IsCanceled);
+            // Check that a warning was logged.
+            Assert.AreEqual(1, _logger.WarningsLogged.Count);
+            Assert.IsTrue(_logger.WarningsLogged[0].StartsWith("Failure sending usage data"));
+            Console.WriteLine(_logger.WarningsLogged[0]);
         }
 
         /// <summary>