diff --git a/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs b/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs index 272ab6c4..4c22ee8d 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs +++ b/FiftyOne.Pipeline.CloudRequestEngine/FlowElements/CloudRequestEngine.cs @@ -234,8 +234,7 @@ protected override void ProcessEngine(IFlowData data, CloudRequestData aspectDat } requestMessage.Content = content; - var request = AddCommonHeadersAndSend(requestMessage); - jsonResult = ProcessResponse(request.Result); + jsonResult = ProcessResponse(AddCommonHeadersAndSend(requestMessage)); } aspectData.JsonResponse = jsonResult; @@ -470,8 +469,7 @@ private void GetCloudProperties() using (var requestMessage = new HttpRequestMessage(HttpMethod.Get, url)) { - var request = AddCommonHeadersAndSend(requestMessage); - jsonResult = ProcessResponse(request.Result); + jsonResult = ProcessResponse(AddCommonHeadersAndSend(requestMessage)); } if (string.IsNullOrEmpty(jsonResult) == false) @@ -498,10 +496,10 @@ private void GetCloudEvidenceKeys() using (var requestMessage = new HttpRequestMessage(HttpMethod.Get, _evidenceKeysEndpoint)) { - var request = AddCommonHeadersAndSend(requestMessage); // Note - Don't check for error messages in the response // as it is a flat JSON array. - jsonResult = ProcessResponse(request.Result, false); + jsonResult = ProcessResponse( + AddCommonHeadersAndSend(requestMessage), false); } if (string.IsNullOrEmpty(jsonResult) == false) @@ -527,7 +525,7 @@ private void GetCloudEvidenceKeys() /// /// The response /// - private Task AddCommonHeadersAndSend( + private HttpResponseMessage AddCommonHeadersAndSend( HttpRequestMessage request) { if (string.IsNullOrEmpty(_cloudRequestOrigin) == false && @@ -537,7 +535,32 @@ private Task AddCommonHeadersAndSend( request.Headers.Add(Constants.ORIGIN_HEADER_NAME, _cloudRequestOrigin); } - return _httpClient.SendAsync(request); + return SendRequestAsync(request); } + + /// + /// Send a request and handle any exception if one is thrown. + /// + /// + /// The request to send + /// + /// + /// The response + /// + private HttpResponseMessage SendRequestAsync( + HttpRequestMessage request) + { + var task = _httpClient.SendAsync(request); + + try + { + return task.Result; + } + catch (Exception ex) + { + throw new CloudRequestException( + Messages.ExceptionCloudResponseFailure, ex); + } + } } } diff --git a/FiftyOne.Pipeline.CloudRequestEngine/Messages.Designer.cs b/FiftyOne.Pipeline.CloudRequestEngine/Messages.Designer.cs index ce845dbf..8e180922 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/Messages.Designer.cs +++ b/FiftyOne.Pipeline.CloudRequestEngine/Messages.Designer.cs @@ -96,6 +96,15 @@ internal static string ExceptionCloudPropertyType { } } + /// + /// Looks up a localized string similar to Error waiting for response from the cloud service.. + /// + internal static string ExceptionCloudResponseFailure { + get { + return ResourceManager.GetString("ExceptionCloudResponseFailure", resourceCulture); + } + } + /// /// Looks up a localized string similar to Failed to load aspect properties for element '{0}'. This is because your resource key does not include access to any properties under '{0}'. For more details on resource keys, see our explainer: https://51degrees.com/documentation/_info__resource_keys.html. /// @@ -114,6 +123,15 @@ internal static string ExceptionResourceKeyNeeded { } } + /// + /// Looks up a localized string similar to Failed to send request. A TaskCanceledException was thrown. The likely cause of this would be a connection timeout.. + /// + internal static string ExceptionTaskCanceledRequest { + get { + return ResourceManager.GetString("ExceptionTaskCanceledRequest", resourceCulture); + } + } + /// /// Looks up a localized string similar to Cloud service at '{0}' returned status code '{1}' with content {2}.. /// diff --git a/FiftyOne.Pipeline.CloudRequestEngine/Messages.resx b/FiftyOne.Pipeline.CloudRequestEngine/Messages.resx index ccf9d622..dc1f50d0 100644 --- a/FiftyOne.Pipeline.CloudRequestEngine/Messages.resx +++ b/FiftyOne.Pipeline.CloudRequestEngine/Messages.resx @@ -129,12 +129,18 @@ Unable to determine the type associated with this property. No c# property found matching definition {0}.{1} and json type '{2}' is not mapped. + + Error waiting for response from the cloud service. + Failed to load aspect properties for element '{0}'. This is because your resource key does not include access to any properties under '{0}'. For more details on resource keys, see our explainer: https://51degrees.com/documentation/_info__resource_keys.html A resource key is required to access the 51Degrees cloud service. Please use the 'SetResourceKey' method to supply your resource key. You can obtain one for free from https://configure.51degrees.com + + Failed to send request. A TaskCanceledException was thrown. The likely cause of this would be a connection timeout. + Cloud service at '{0}' returned status code '{1}' with content {2}. diff --git a/FiftyOne.Pipeline.Core/FlowElements/PipelineBuilderBase.cs b/FiftyOne.Pipeline.Core/FlowElements/PipelineBuilderBase.cs index 059402cc..526548a2 100644 --- a/FiftyOne.Pipeline.Core/FlowElements/PipelineBuilderBase.cs +++ b/FiftyOne.Pipeline.Core/FlowElements/PipelineBuilderBase.cs @@ -60,7 +60,7 @@ public abstract class PipelineBuilderBase /// If true then Pipeline will call Dispose on its child elements /// when it is disposed. /// - private bool _autoDisposeElements = false; + private bool _autoDisposeElements = true; /// /// If true then Pipeline will suppress exceptions added to diff --git a/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/CloudRequestEngineTests.cs b/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/CloudRequestEngineTests.cs index 21029fc9..a3a131a5 100644 --- a/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/CloudRequestEngineTests.cs +++ b/Tests/FiftyOne.Pipeline.CloudRequestEngine.Tests/CloudRequestEngineTests.cs @@ -33,6 +33,7 @@ using System.Linq; using System.Net; using System.Net.Http; +using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -836,19 +837,57 @@ public void ValidateErrorHandling_HttpDataSetInException() Assert.IsNotNull(ex.ResponseHeaders, "Response headers not populated"); Assert.IsTrue(ex.ResponseHeaders.Count > 0, "Response headers not populated"); } + } + + /// + /// Verify that an exception throw by the task that is returned by HttpClient.SendAsync + /// will be handled and wrapped in nice informative CloudRequestException. + /// + [TestMethod] + public void ValidateErrorHandling_ExceptionInRequestTask() + { + string resourceKey = "resource_key"; + string userAgent = "iPhone"; + Exception exception = null; + + ConfigureMockedClient(r => true, true); + var engine = new CloudRequestEngineBuilder(_loggerFactory, _httpClient) + .SetResourceKey(resourceKey) + .Build(); + + try + { + using (var pipeline = new PipelineBuilder(_loggerFactory).AddFlowElement(engine).Build()) + { + var data = pipeline.CreateFlowData(); + data.AddEvidence("query.User-Agent", userAgent); + data.Process(); + } + } + catch (Exception ex) + { + exception = ex; + } + Assert.IsNotNull(exception, "Expected exception to occur"); + Assert.IsInstanceOfType(exception, typeof(AggregateException)); + var aggEx = exception as AggregateException; + Assert.AreEqual(aggEx.InnerExceptions.Count, 1); + var realEx = aggEx.InnerExceptions[0]; + Assert.IsInstanceOfType(realEx, typeof(CloudRequestException)); } /// /// Setup _httpClient to respond with the configured messages. /// private void ConfigureMockedClient( - Func expectedJsonParameters) + Func expectedJsonParameters, + bool throwExceptionOnJsonRequest = false) { // ARRANGE _handlerMock = new Mock(MockBehavior.Strict); // Set up the JSON response. - _handlerMock + var setup = _handlerMock .Protected() // Setup the PROTECTED method to mock .Setup>( @@ -856,14 +895,27 @@ private void ConfigureMockedClient( ItExpr.Is(r => expectedJsonParameters(r) && r.RequestUri.AbsolutePath.ToLower().EndsWith("json")), ItExpr.IsAny() - ) - // prepare the expected response of the mocked http call - .ReturnsAsync(new HttpResponseMessage() - { - StatusCode = HttpStatusCode.OK, - Content = new StringContent(_jsonResponse), - }) + ); + + if (throwExceptionOnJsonRequest) + { + // Configure the call to the json endpoint to throw an exception. + var task = new Task(() => throw new Exception("TEST")); + // We have to start the task or it will never actually run! + task.Start(); + setup.Returns(task); + } + else + { + // Prepare the expected response of the mocked http call + setup.ReturnsAsync(new HttpResponseMessage() + { + StatusCode = HttpStatusCode.OK, + Content = new StringContent(_jsonResponse), + }) .Verifiable(); + } + // Set up the evidencekeys response. _handlerMock .Protected() diff --git a/ci/common-ci b/ci/common-ci index 43de1ade..f8fa48b8 160000 --- a/ci/common-ci +++ b/ci/common-ci @@ -1 +1 @@ -Subproject commit 43de1ade5e414cc4830ad2d24dc5434e341fec3c +Subproject commit f8fa48b81b5ff6a4ac3045f165515e0ab4d6e745