Skip to content

Commit adf9838

Browse files
authored
feat: Clean up A2ACardResolver (#28)
* feat: Clean up A2ACardResolver - Support using a shared HttpClient - Implement GetAgentCard with synchronous APIs on .NET Core - Avoid response buffering - Use HttpStatusCode enum rather than int - Specify ConfigureAwait - Avoid unnecessary null checks on _logger * Address feedback
1 parent b267755 commit adf9838

File tree

4 files changed

+51
-41
lines changed

4 files changed

+51
-41
lines changed

src/A2A/Client/A2ACardResolver.cs

Lines changed: 39 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
using Microsoft.Extensions.Logging;
22
using Microsoft.Extensions.Logging.Abstractions;
3+
using System.Diagnostics;
4+
using System.Net;
35
using System.Text.Json;
46

57
namespace A2A;
@@ -9,30 +11,34 @@ namespace A2A;
911
/// </summary>
1012
public sealed class A2ACardResolver
1113
{
14+
private readonly HttpClient _httpClient;
15+
private readonly Uri _agentCardPath;
16+
private readonly ILogger _logger;
17+
1218
/// <summary>
1319
/// Creates a new instance of the A2ACardResolver
1420
/// </summary>
15-
/// <param name="httpClient">Optional HTTP client (if not provided, a new one will be created)</param>
16-
/// <param name="agentCardPath">Path to the agent card (defaults to /.well-known/agent.json)</param>
21+
/// <param name="httpClient">Optional HTTP client (if not provided, a shared one will be used)</param>
22+
/// <param name="agentCardPath">Path to the agent card (defaults to "/.well-known/agent.json")</param>
1723
/// <param name="logger">Optional logger</param>
1824
public A2ACardResolver(
19-
HttpClient httpClient,
25+
HttpClient? httpClient,
2026
string agentCardPath = "/.well-known/agent.json",
2127
ILogger? logger = null)
2228
{
23-
_agentCardPath = agentCardPath.TrimStart('/');
24-
_httpClient = httpClient;
25-
_httpClient.Timeout = TimeSpan.FromSeconds(30); // Set a reasonable timeout
29+
if (agentCardPath is null)
30+
{
31+
throw new ArgumentNullException(nameof(agentCardPath), "Agent card path cannot be null.");
32+
}
33+
34+
_httpClient = httpClient ?? A2AClient.s_sharedClient;
35+
_agentCardPath = new Uri(agentCardPath.TrimStart('/'), UriKind.RelativeOrAbsolute);
2636
_logger = logger ?? NullLogger.Instance;
27-
}
2837

29-
/// <summary>
30-
/// Gets the agent card synchronously
31-
/// </summary>
32-
/// <returns>The agent card</returns>
33-
public AgentCard GetAgentCard()
34-
{
35-
return GetAgentCardAsync().GetAwaiter().GetResult();
38+
if (_httpClient.BaseAddress is null && !_agentCardPath.IsAbsoluteUri)
39+
{
40+
throw new ArgumentException($"HttpClient.BaseAddress must be set when using a relative {nameof(agentCardPath)}.", nameof(httpClient));
41+
}
3642
}
3743

3844
/// <summary>
@@ -42,44 +48,44 @@ public AgentCard GetAgentCard()
4248
/// <returns>The agent card</returns>
4349
public async Task<AgentCard> GetAgentCardAsync(CancellationToken cancellationToken = default)
4450
{
45-
string url = $"{_httpClient.BaseAddress}/{_agentCardPath}";
46-
_logger?.LogInformation("Fetching agent card from {Url}", url);
51+
if (_logger.IsEnabled(LogLevel.Information))
52+
{
53+
Debug.Assert(_agentCardPath.IsAbsoluteUri || _httpClient.BaseAddress is not null);
54+
_logger.LogInformation("Fetching agent card from '{Url}'", _agentCardPath.IsAbsoluteUri ?
55+
_agentCardPath :
56+
new Uri(_httpClient.BaseAddress!, _agentCardPath.ToString()));
57+
}
4758

4859
try
4960
{
50-
var response = await _httpClient.GetAsync(_agentCardPath, cancellationToken);
61+
using var response = await _httpClient.GetAsync(_agentCardPath, HttpCompletionOption.ResponseHeadersRead, cancellationToken).ConfigureAwait(false);
5162

5263
response.EnsureSuccessStatusCode();
5364

54-
var content = await response.Content.ReadAsStreamAsync(
55-
#if NET8_0_OR_GREATER
65+
using var responseStream = await response.Content.ReadAsStreamAsync(
66+
#if NET
5667
cancellationToken
5768
#endif
58-
);
69+
).ConfigureAwait(false);
5970

60-
return JsonSerializer.Deserialize(content, A2AJsonUtilities.JsonContext.Default.AgentCard) ??
71+
return JsonSerializer.Deserialize(responseStream, A2AJsonUtilities.JsonContext.Default.AgentCard) ??
6172
throw new A2AClientJsonException("Failed to parse agent card JSON.");
6273
}
6374
catch (JsonException ex)
6475
{
65-
_logger?.LogError(ex, "Failed to parse agent card JSON");
76+
_logger.LogError(ex, "Failed to parse agent card JSON");
6677
throw new A2AClientJsonException($"Failed to parse JSON: {ex.Message}");
6778
}
6879
catch (HttpRequestException ex)
6980
{
70-
#if NET8_0_OR_GREATER
71-
int statusCode = (int)(ex.StatusCode ?? System.Net.HttpStatusCode.InternalServerError);
72-
#else
73-
int statusCode = (int)System.Net.HttpStatusCode.InternalServerError;
81+
HttpStatusCode statusCode =
82+
#if NET
83+
ex.StatusCode ??
7484
#endif
75-
_logger?.LogError(ex, "HTTP request failed with status code {StatusCode}", statusCode);
85+
HttpStatusCode.InternalServerError;
86+
87+
_logger.LogError(ex, "HTTP request failed with status code {StatusCode}", statusCode);
7688
throw new A2AClientHTTPException(statusCode, ex.Message);
7789
}
7890
}
79-
80-
#region private
81-
private readonly HttpClient _httpClient;
82-
private readonly string _agentCardPath;
83-
private readonly ILogger _logger;
84-
#endregion
8591
}

src/A2A/Client/A2AClient.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ namespace A2A;
77

88
public sealed class A2AClient : IA2AClient
99
{
10-
private static readonly HttpClient s_sharedClient = new();
10+
internal static readonly HttpClient s_sharedClient = new();
1111

1212
private readonly HttpClient _httpClient;
1313

src/A2A/JsonRpc/ErrorTypes.cs

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace A2A;
1+
using System.Net;
2+
3+
namespace A2A;
24

35
/// <summary>
46
/// Base exception for A2A client errors
@@ -24,7 +26,7 @@ public class A2AClientHTTPException : A2AClientException
2426
/// <summary>
2527
/// The HTTP status code
2628
/// </summary>
27-
public int StatusCode { get; }
29+
public HttpStatusCode StatusCode { get; }
2830

2931
/// <summary>
3032
/// The error message
@@ -36,8 +38,8 @@ public class A2AClientHTTPException : A2AClientException
3638
/// </summary>
3739
/// <param name="statusCode">The HTTP status code</param>
3840
/// <param name="message">The error message</param>
39-
public A2AClientHTTPException(int statusCode, string message)
40-
: base($"HTTP Error {statusCode}: {message}")
41+
public A2AClientHTTPException(HttpStatusCode statusCode, string message)
42+
: base($"HTTP Error {(int)statusCode}: {message}")
4143
{
4244
StatusCode = statusCode;
4345
ErrorMessage = message;

tests/A2A.UnitTests/JsonRpc/ErrorTypesTests.cs

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,6 @@
1-
namespace A2A.UnitTests.JsonRpc;
1+
using System.Net;
2+
3+
namespace A2A.UnitTests.JsonRpc;
24

35
public class ErrorTypesTests
46
{
@@ -20,10 +22,10 @@ public void A2AClientError_ConstructsWithMessageAndInnerException()
2022
public void A2AClientHTTPError_PropertiesSetCorrectly()
2123
{
2224
// Act
23-
var sut = new A2AClientHTTPException(404, "Not Found");
25+
var sut = new A2AClientHTTPException(HttpStatusCode.NotFound, "Not Found");
2426

2527
// Assert
26-
Assert.Equal(404, sut.StatusCode);
28+
Assert.Equal(HttpStatusCode.NotFound, sut.StatusCode);
2729
Assert.Equal("Not Found", sut.ErrorMessage);
2830
Assert.Contains("404", sut.Message);
2931
Assert.Contains("Not Found", sut.Message);

0 commit comments

Comments
 (0)