Skip to content

Commit

Permalink
fix: Refactor to eliminate usage of .GetAwaiter().GetResult() in Fr…
Browse files Browse the repository at this point in the history
…amework builds. (#2534) (#2535)
  • Loading branch information
tippmar-nr committed Jun 10, 2024
1 parent 227003a commit cfb2c28
Show file tree
Hide file tree
Showing 15 changed files with 68 additions and 77 deletions.
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System;
Expand All @@ -23,7 +23,7 @@ protected HttpClientBase(IWebProxy proxy)
_proxy = proxy;
}

public abstract Task<IHttpResponse> SendAsync(IHttpRequest request);
public abstract IHttpResponse Send(IHttpRequest request);

public virtual void Dispose()
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

#if !NETFRAMEWORK
using System.IO;
using System.Net.Http;
using System.Threading.Tasks;
using NewRelic.Agent.Core.DataTransport.Client.Interfaces;

namespace NewRelic.Agent.Core.DataTransport.Client
Expand All @@ -21,9 +20,9 @@ public HttpContentWrapper(HttpContent httpContent)
_httpContent = httpContent;
}

public Task<Stream> ReadAsStreamAsync()
public Stream ReadAsStream()
{
return _httpContent.ReadAsStreamAsync();
return _httpContent.ReadAsStreamAsync().GetAwaiter().GetResult();
}

public IHttpContentHeadersWrapper Headers => new HttpContentHeadersWrapper(_httpContent.Headers);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

#if !NETFRAMEWORK
Expand Down Expand Up @@ -28,7 +28,7 @@ public HttpResponse(Guid requestGuid, IHttpResponseMessageWrapper httpResponseMe
_httpResponseMessageWrapper = httpResponseMessageWrapper;
}

public async Task<string> GetContentAsync()
public string GetContent()
{
try
{
Expand All @@ -37,7 +37,7 @@ public async Task<string> GetContentAsync()
return Constants.EmptyResponseBody;
}

var responseStream = await _httpResponseMessageWrapper.Content.ReadAsStreamAsync();
var responseStream = _httpResponseMessageWrapper.Content.ReadAsStream();

var contentTypeEncoding = _httpResponseMessageWrapper.Content.Headers.ContentEncoding;
if (contentTypeEncoding.Contains("gzip"))
Expand All @@ -48,7 +48,7 @@ public async Task<string> GetContentAsync()
using (responseStream)
using (var reader = new StreamReader(responseStream, Encoding.UTF8))
{
var responseBody = await reader.ReadLineAsync();
var responseBody = reader.ReadLineAsync().GetAwaiter().GetResult();

if (responseBody != null)
{
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System;
Expand All @@ -8,6 +8,6 @@ namespace NewRelic.Agent.Core.DataTransport.Client.Interfaces
{
public interface IHttpClient : IDisposable
{
Task<IHttpResponse> SendAsync(IHttpRequest request);
IHttpResponse Send(IHttpRequest request);
}
}
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System.IO;
using System.Threading.Tasks;

namespace NewRelic.Agent.Core.DataTransport.Client.Interfaces
{
Expand All @@ -12,7 +11,7 @@ namespace NewRelic.Agent.Core.DataTransport.Client.Interfaces
/// </summary>
public interface IHttpContentWrapper
{
Task<Stream> ReadAsStreamAsync();
Stream ReadAsStream();
IHttpContentHeadersWrapper Headers { get; }
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

using System;
Expand All @@ -11,6 +11,6 @@ public interface IHttpResponse : IDisposable
{
bool IsSuccessStatusCode { get; }
HttpStatusCode StatusCode { get; }
Task<string> GetContentAsync();
string GetContent();
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

#if !NETFRAMEWORK
Expand Down Expand Up @@ -32,7 +32,7 @@ public NRHttpClient(IWebProxy proxy, IConfiguration configuration) : base(proxy)
_httpClientWrapper = new HttpClientWrapper(httpClient, (int)configuration.CollectorTimeout);
}

public override async Task<IHttpResponse> SendAsync(IHttpRequest request)
public override IHttpResponse Send(IHttpRequest request)
{
try
{
Expand Down Expand Up @@ -65,7 +65,7 @@ public override async Task<IHttpResponse> SendAsync(IHttpRequest request)
req.Content.Headers.Add(contentHeader.Key, contentHeader.Value);
}

var response = await _httpClientWrapper.SendAsync(req);
var response = _httpClientWrapper.SendAsync(req).GetAwaiter().GetResult();

var httpResponse = new HttpResponse(request.RequestGuid, response);
return httpResponse;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0

#if NETFRAMEWORK
Expand All @@ -25,7 +25,7 @@ public NRWebRequestClient(IWebProxy proxy, IConfiguration configuration) : base(
_configuration = configuration;
}

public override async Task<IHttpResponse> SendAsync(IHttpRequest request)
public override IHttpResponse Send(IHttpRequest request)
{
try
{
Expand Down Expand Up @@ -61,12 +61,10 @@ public override async Task<IHttpResponse> SendAsync(IHttpRequest request)
throw new NullReferenceException("outputStream");
}

// .ConfigureAwait(false) is required here for some reason
await outputStream.WriteAsync(request.Content.PayloadBytes, 0, (int)_httpWebRequest.ContentLength).ConfigureAwait(false);
outputStream.Write(request.Content.PayloadBytes, 0, (int)_httpWebRequest.ContentLength);
}

// .ConfigureAwait(false) is required here for some reason
var resp = (HttpWebResponse)await _httpWebRequest.GetResponseAsync().ConfigureAwait(false);
var resp = (HttpWebResponse)_httpWebRequest.GetResponse();

return new WebRequestClientResponse(request.RequestGuid, resp);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
// Copyright 2020 New Relic, Inc. All rights reserved.
// Copyright 2020 New Relic, Inc. All rights reserved.
// SPDX-License-Identifier: Apache-2.0
#if NETFRAMEWORK
using System;
Expand Down Expand Up @@ -26,7 +26,7 @@ public WebRequestClientResponse(Guid requestGuid, HttpWebResponse response)
_response = response;
}

public Task<string> GetContentAsync()
public string GetContent()
{
try
{
Expand All @@ -51,14 +51,14 @@ public Task<string> GetContentAsync()
using (var reader = new StreamReader(responseStream, Encoding.UTF8))
{
var responseBody = reader.ReadLine();
return Task.FromResult(responseBody ?? Constants.EmptyResponseBody);
return responseBody ?? Constants.EmptyResponseBody;
}
}
catch (Exception ex)
{
Log.Error(ex, "Request({0}): Unable to parse response body.", _requestGuid);

return Task.FromResult(Constants.EmptyResponseBody);
return Constants.EmptyResponseBody;
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,9 @@ public string SendData(string method, ConnectionInfo connectionInfo, string seri
foreach (var header in _requestHeadersMap)
request.Headers.Add(header.Key, header.Value);

using var response = httpClient.SendAsync(request).GetAwaiter().GetResult();
using var response = httpClient.Send(request);

var responseContent = response.GetContentAsync().GetAwaiter().GetResult();
var responseContent = response.GetContent();

if (!response.IsSuccessStatusCode)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@
#if !NETFRAMEWORK
using System;
using System.Collections.Generic;
using System.Net.Http;
using System.IO;
using System.IO.Compression;
using Telerik.JustMock;
using NUnit.Framework;
using System.Net;
using System.Text;
using System.Threading.Tasks;
using NewRelic.Agent.Core.DataTransport.Client.Interfaces;
using Telerik.JustMock.Helpers;

Expand Down Expand Up @@ -40,30 +38,30 @@ public void TearDown()
}

[Test]
public async Task GetContentAsync_ReturnsEmptyResponseBody_WhenContentIsNull()
public void GetContent_ReturnsEmptyResponseBody_WhenContentIsNull()
{
_mockHttpResponseMessage.Arrange(message => message.Content).Returns((IHttpContentWrapper)null);

var result = await _httpResponse.GetContentAsync();
var result = _httpResponse.GetContent();

Assert.That(result, Is.EqualTo(Constants.EmptyResponseBody));
}

[Test]
public async Task GetContentAsync_ReturnsContent_WhenContentIsNotNull()
public void GetContent_ReturnsContent_WhenContentIsNotNull()
{
var mockContent = Mock.Create<IHttpContentWrapper>();
var stream = new MemoryStream(Encoding.UTF8.GetBytes(TestResponseBody));
_mockHttpResponseMessage.Arrange(message => message.Content).Returns(mockContent);
mockContent.Arrange(content => content.ReadAsStreamAsync()).ReturnsAsync((Stream)stream);
mockContent.Arrange(content => content.ReadAsStream()).Returns(stream);

var result = await _httpResponse.GetContentAsync();
var result = _httpResponse.GetContent();

Assert.That(result, Is.EqualTo(TestResponseBody));
}

[Test]
public async Task GetContentAsync_HandlesGzipDecompression_WhenContentEncodingIsGzip()
public void GetContent_HandlesGzipDecompression_WhenContentEncodingIsGzip()
{
var compressedStream = new MemoryStream();
using (var gzipStream = new GZipStream(compressedStream, CompressionMode.Compress, true))
Expand All @@ -79,9 +77,9 @@ public async Task GetContentAsync_HandlesGzipDecompression_WhenContentEncodingIs
var mockHeaders = Mock.Create<IHttpContentHeadersWrapper>();
mockContent.Arrange(content => content.Headers).Returns(mockHeaders);
mockHeaders.Arrange(headers => headers.ContentEncoding).Returns(new List<string> { "gzip" });
mockContent.Arrange(content => content.ReadAsStreamAsync()).ReturnsAsync((Stream)compressedStream);
mockContent.Arrange(content => content.ReadAsStream()).Returns(compressedStream);

var result = await _httpResponse.GetContentAsync();
var result = _httpResponse.GetContent();

Assert.That(result, Is.EqualTo(TestResponseBody));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public void TearDown()
_mockHttpClientWrapper.Dispose();
}
[Test]
public async Task SendAsync_ReturnsResponse_WhenSendAsyncSucceeds()
public void Send_ReturnsResponse_WhenSendAsyncSucceeds()
{
// Arrange
var request = CreateHttpRequest();
Expand All @@ -64,7 +64,7 @@ public async Task SendAsync_ReturnsResponse_WhenSendAsyncSucceeds()
.ReturnsAsync(mockHttpResponseMessage);

// Act
var response = await _client.SendAsync(request);
var response = _client.Send(request);

// Assert
Assert.That(response, Is.Not.Null);
Expand All @@ -80,7 +80,7 @@ public void SendAsync_ThrowsHttpRequestException_WhenSendAsyncThrows()
.Throws<HttpRequestException>();

// Act & Assert
Assert.ThrowsAsync<HttpRequestException>(() => _client.SendAsync(request));
Assert.Throws<HttpRequestException>(() => _client.Send(request));
}

private IHttpRequest CreateHttpRequest()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,10 @@
using System;
using System.IO;
using System.Net;
using System.Threading.Tasks;
using NewRelic.Agent.Configuration;
using NewRelic.Agent.Core.DataTransport.Client.Interfaces;
using NUnit.Framework;
using Telerik.JustMock;
using Telerik.JustMock.AutoMock.Ninject.Activation;
using Telerik.JustMock.Helpers;

namespace NewRelic.Agent.Core.DataTransport.Client
{
Expand Down Expand Up @@ -52,20 +49,20 @@ public void TearDown()
}

[Test]
public async Task SendAsync_ShouldReturnValidResponse_WhenWebRequestIsSuccessful()
public void Send_ShouldReturnValidResponse_WhenWebRequestIsSuccessful()
{
// Arrange
var fakeResponse = Mock.Create<HttpWebResponse>();
_client.SetHttpWebRequestFunc(uri =>
{
var mockWebRequest = Mock.Create<HttpWebRequest>();
Mock.Arrange(() => mockWebRequest.GetRequestStream()).Returns(new MemoryStream());
Mock.Arrange(() => mockWebRequest.GetResponseAsync()).ReturnsAsync((WebResponse)fakeResponse);
Mock.Arrange(() => mockWebRequest.GetResponse()).Returns((WebResponse)fakeResponse);
return mockWebRequest;
});

// Act
var response = await _client.SendAsync(_request);
var response = _client.Send(_request);

// Assert
Assert.That(response, Is.Not.Null);
Expand All @@ -83,7 +80,7 @@ public void SendAsync_ShouldThrow_WhenNullOutputStream()
});

// Act & Assert
Assert.ThrowsAsync<NullReferenceException>(() => _client.SendAsync(_request));
Assert.Throws<NullReferenceException>(() => _client.Send(_request));
}

[Test]
Expand All @@ -95,15 +92,15 @@ public void SendAsync_ThrowsWebException_WhenWebExceptionResponseIsNull()
var mockWebRequest = Mock.Create<HttpWebRequest>();
Mock.Arrange(() => mockWebRequest.Address).Returns(new Uri("https://sometesthost.com"));
var webException = new WebException("testing");
Mock.Arrange(() => mockWebRequest.GetResponseAsync()).Throws(webException);
Mock.Arrange(() => mockWebRequest.GetResponse()).Throws(webException);
return mockWebRequest;
});

// Act & Assert
Assert.ThrowsAsync<WebException>(() => _client.SendAsync(_request));
Assert.Throws<WebException>(() => _client.Send(_request));
}
[Test]
public async Task SendAsync_ReturnsResponse_WhenWebExceptionResponseIsNotNull()
public void Send_ReturnsResponse_WhenWebExceptionResponseIsNotNull()
{
// Arrange
_client.SetHttpWebRequestFunc(uri =>
Expand All @@ -115,12 +112,12 @@ public async Task SendAsync_ReturnsResponse_WhenWebExceptionResponseIsNotNull()
Mock.Arrange(() => mockHttpWebResponse.StatusCode).Returns(HttpStatusCode.BadRequest);
Mock.Arrange(() => mockHttpWebResponse.StatusDescription).Returns("Bad Request");
var webException = new WebException("testing", null, WebExceptionStatus.SendFailure,mockHttpWebResponse);
Mock.Arrange(() => mockWebRequest.GetResponseAsync()).Throws(webException);
Mock.Arrange(() => mockWebRequest.GetResponse()).Throws(webException);
return mockWebRequest;
});

// Act
var response = await _client.SendAsync(_request);
var response = _client.Send(_request);

// Assert
Assert.That(response, Is.Not.Null);
Expand Down
Loading

0 comments on commit cfb2c28

Please sign in to comment.