Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ private async ValueTask<TResponse> CallProductEndpointCoreAsync<TResponse>(
)
where TResponse : TransportResponse, new()
{
if (callDetails?.HasSuccessfulStatusCodeAndExpectedContentType ?? false)
if (_productRegistration.IsValidResponse(callDetails))
return null;

var pipelineFailure = callDetails?.HttpStatusCode != null ? PipelineFailure.BadResponse : PipelineFailure.BadRequest;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,35 @@ public override bool NodePredicate(Node node) =>
/// We consider all status codes &gt;= 200 and &lt; 300 valid by default.
/// Elasticsearch might return 404 for valid responses in some cases (e.g. `GET /my-index/_doc/missing-doc-id`) but also for actual error cases like
/// missing endpoints, missing indices (e.g. `GET /missing-index/_mapping`), etc.
/// The 404 case is handled on a per-request basis (see <see cref="ElasticsearchResponseHelper.IsValidResponse"/> for details).
/// The 404 case is handled on a per-request basis (see <see cref="IsValidResponse(ApiCallDetails)"/> for details).
/// </remarks>
public override bool HttpStatusCodeClassifier(HttpMethod method, int statusCode) =>
statusCode is >= 200 and < 300;

/// <inheritdoc cref="ProductRegistration.IsValidResponse(ApiCallDetails)"/>
/// <remarks>
/// A response is valid when:
/// <list type="bullet">
/// <item>The content-type matches what the caller asked for.</item>
/// <item>For 404 responses: there is no extracted Elasticsearch server error
/// (404s without an error body — e.g. <c>GET /my-index/_doc/missing-doc-id</c> — represent
/// a legitimate "missing entity" rather than a failure).</item>
/// <item>Otherwise: the status code is in the success range (or explicitly allowed via
/// <see cref="IRequestConfiguration.AllowedStatusCodes"/>).</item>
/// </list>
/// </remarks>
public override bool IsValidResponse(ApiCallDetails? details)
{
if (details is null || !details.HasExpectedContentType)
return false;

var serverError = details.ProductError as ElasticsearchServerError;
if (details.HttpStatusCode is 404)
return !serverError?.HasError() ?? true;

return details.HasSuccessfulStatusCode;
}

/// <inheritdoc cref="ProductRegistration.TryGetServerErrorReason{TResponse}"/>>
public override bool TryGetServerErrorReason<TResponse>(TResponse response, out string? reason)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public ElasticsearchDynamicResponse(DynamicDictionary dictionary) : base(diction
public ElasticsearchServerError? ElasticsearchServerError => ElasticsearchResponseHelper.GetElasticsearchError(ApiCallDetails);

/// <inheritdoc />
public bool IsValidResponse => ElasticsearchResponseHelper.IsValidResponse(ApiCallDetails);
public bool IsValidResponse =>
ApiCallDetails?.TransportConfiguration?.ProductRegistration?.IsValidResponse(ApiCallDetails) ?? false;

/// <inheritdoc />
public IEnumerable<string> ElasticsearchWarnings => ElasticsearchResponseHelper.GetElasticsearchWarnings(ApiCallDetails);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ public ElasticsearchJsonResponse(JsonNode node) : base(node) { }
public ElasticsearchServerError? ElasticsearchServerError => ElasticsearchResponseHelper.GetElasticsearchError(ApiCallDetails);

/// <inheritdoc />
public bool IsValidResponse => ElasticsearchResponseHelper.IsValidResponse(ApiCallDetails);
public bool IsValidResponse =>
ApiCallDetails?.TransportConfiguration?.ProductRegistration?.IsValidResponse(ApiCallDetails) ?? false;

/// <inheritdoc />
public IEnumerable<string> ElasticsearchWarnings => ElasticsearchResponseHelper.GetElasticsearchWarnings(ApiCallDetails);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ public ElasticsearchPipeResponse(Stream responseStream, string? contentType) : b
public ElasticsearchServerError? ElasticsearchServerError => ElasticsearchResponseHelper.GetElasticsearchError(ApiCallDetails);

/// <inheritdoc />
public bool IsValidResponse => ElasticsearchResponseHelper.IsValidResponse(ApiCallDetails);
public bool IsValidResponse =>
ApiCallDetails?.TransportConfiguration?.ProductRegistration?.IsValidResponse(ApiCallDetails) ?? false;

/// <inheritdoc />
public IEnumerable<string> ElasticsearchWarnings => ElasticsearchResponseHelper.GetElasticsearchWarnings(ApiCallDetails);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ public abstract class ElasticsearchResponse : TransportResponse, IElasticsearchR
/// <inheritdoc />
[JsonIgnore]
public virtual bool IsValidResponse =>
ElasticsearchResponseHelper.IsValidResponse(ApiCallDetails);
ApiCallDetails?.TransportConfiguration?.ProductRegistration?.IsValidResponse(ApiCallDetails) ?? false;

/// <inheritdoc />
[JsonIgnore]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,6 @@ namespace Elastic.Transport.Products.Elasticsearch;
/// </summary>
internal static class ElasticsearchResponseHelper
{
public static bool IsValidResponse(ApiCallDetails? apiCallDetails)
{
if (apiCallDetails is null || !apiCallDetails.HasExpectedContentType)
return false;

// Elasticsearch returns 404 for valid responses in some cases (e.g. `GET /my-index/_doc/missing-doc-id`) but also for actual error cases like
// missing endpoints, missing indices (e.g. `GET /missing-index/_mapping`), etc.
// We consider all status codes >= 200 and < 300 valid by default. For 404, we assume "invalid" and try to parse the Elasticsearch
// error response from the body.
// A 404 status code without an error body indicates a valid response.

var serverError = GetElasticsearchError(apiCallDetails);
if (apiCallDetails.HttpStatusCode is 404)
return !serverError?.HasError() ?? true;

return apiCallDetails.HasSuccessfulStatusCode;
}

public static ElasticsearchServerError? GetElasticsearchError(ApiCallDetails? apiCallDetails) =>
apiCallDetails?.ProductError as ElasticsearchServerError;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ public ElasticsearchStreamResponse(Stream body, string? contentType) : base(body
public ElasticsearchServerError? ElasticsearchServerError => ElasticsearchResponseHelper.GetElasticsearchError(ApiCallDetails);

/// <inheritdoc />
public bool IsValidResponse => ElasticsearchResponseHelper.IsValidResponse(ApiCallDetails);
public bool IsValidResponse =>
ApiCallDetails?.TransportConfiguration?.ProductRegistration?.IsValidResponse(ApiCallDetails) ?? false;

/// <inheritdoc />
public IEnumerable<string> ElasticsearchWarnings => ElasticsearchResponseHelper.GetElasticsearchWarnings(ApiCallDetails);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ public ElasticsearchStringResponse(string body) : base(body) { }
public ElasticsearchServerError? ElasticsearchServerError => ElasticsearchResponseHelper.GetElasticsearchError(ApiCallDetails);

/// <inheritdoc />
public bool IsValidResponse => ElasticsearchResponseHelper.IsValidResponse(ApiCallDetails);
public bool IsValidResponse =>
ApiCallDetails?.TransportConfiguration?.ProductRegistration?.IsValidResponse(ApiCallDetails) ?? false;

/// <inheritdoc />
public IEnumerable<string> ElasticsearchWarnings => ElasticsearchResponseHelper.GetElasticsearchWarnings(ApiCallDetails);
Expand Down
12 changes: 12 additions & 0 deletions src/Elastic.Transport/Products/ProductRegistration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -162,4 +162,16 @@ public abstract class ProductRegistration
/// <param name="responseStream">A seekable stream containing the response body.</param>
/// <returns>An <see cref="ErrorResponse"/> if one was successfully extracted; otherwise <c>null</c>.</returns>
public virtual ErrorResponse? TryExtractError(BoundConfiguration boundConfiguration, Stream responseStream) => null;

/// <summary>
/// Whether the response should be treated as valid for the caller — meaning no exception is
/// raised under <see cref="IRequestConfiguration.ThrowExceptions"/>, and the product-level
/// <c>IsValidResponse</c> reads <c>true</c>.
/// <para>Default: a successful HTTP status code with the expected content type. Products override
/// to encode their own semantics (e.g. Elasticsearch treats a 404 with no extracted server error
/// as valid because it represents a missing entity rather than a true failure).</para>
/// </summary>
/// <param name="details">The <see cref="ApiCallDetails"/> for the response.</param>
public virtual bool IsValidResponse(ApiCallDetails? details) =>
details?.HasSuccessfulStatusCodeAndExpectedContentType ?? false;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
// Licensed to Elasticsearch B.V under one or more agreements.
// Elasticsearch B.V licenses this file to you under the Apache 2.0 License.
// See the LICENSE file in the project root for more information

using Elastic.Transport.Products;
using Elastic.Transport.Products.Elasticsearch;
using FluentAssertions;
using Xunit;

namespace Elastic.Transport.Tests.Products;

public class ProductRegistrationIsValidResponseTests
{
[Fact]
public void DefaultRegistrationReturnsFalseForNullDetails() =>
DefaultProductRegistration.Default.IsValidResponse(null).Should().BeFalse();

[Fact]
public void DefaultRegistrationReturnsTrueForSuccessAndExpectedContentType() =>
DefaultProductRegistration.Default
.IsValidResponse(MakeDetails(200, hasSuccess: true))
.Should().BeTrue();

[Fact]
public void DefaultRegistrationReturnsFalseForNonSuccess() =>
DefaultProductRegistration.Default
.IsValidResponse(MakeDetails(404, hasSuccess: false))
.Should().BeFalse();

[Fact]
public void DefaultRegistrationReturnsFalseForUnexpectedContentType()
{
var details = MakeDetails(200, hasSuccess: true);
details.HasExpectedContentType = false;
DefaultProductRegistration.Default.IsValidResponse(details).Should().BeFalse();
}

[Fact]
public void ElasticsearchReturnsTrueFor404WithNoExtractedServerError() =>
ElasticsearchProductRegistration.Default
.IsValidResponse(MakeDetails(404, hasSuccess: false))
.Should().BeTrue();

[Fact]
public void ElasticsearchReturnsFalseFor404WithExtractedServerError()
{
var details = MakeDetails(404, hasSuccess: false);
details.ProductError = new ElasticsearchServerError(
new Error { Type = "index_not_found_exception", Reason = "no such index" },
statusCode: 404);
ElasticsearchProductRegistration.Default.IsValidResponse(details).Should().BeFalse();
}

[Fact]
public void ElasticsearchReturnsFalseFor500WithExtractedServerError()
{
var details = MakeDetails(500, hasSuccess: false);
details.ProductError = new ElasticsearchServerError(
new Error { Type = "internal", Reason = "boom" },
statusCode: 500);
ElasticsearchProductRegistration.Default.IsValidResponse(details).Should().BeFalse();
}

[Fact]
public void ElasticsearchReturnsTrueForSuccess() =>
ElasticsearchProductRegistration.Default
.IsValidResponse(MakeDetails(200, hasSuccess: true))
.Should().BeTrue();

[Fact]
public void ElasticsearchReturnsFalseFor404WithUnexpectedContentType()
{
var details = MakeDetails(404, hasSuccess: false);
details.HasExpectedContentType = false;
ElasticsearchProductRegistration.Default.IsValidResponse(details).Should().BeFalse();
}

private static ApiCallDetails MakeDetails(int statusCode, bool hasSuccess) =>
new()
{
HttpStatusCode = statusCode,
HasSuccessfulStatusCode = hasSuccess,
HasExpectedContentType = true,
};
}
Loading