Align ThrowExceptions decision with ProductRegistration.IsValidResponse (#208)

Author: flobernd

src/Elastic.Transport/Components/Pipeline/RequestPipeline.cs

@@ -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;

src/Elastic.Transport/Products/Elasticsearch/ElasticsearchProductRegistration.cs

@@ -164,11 +164,35 @@ public override bool NodePredicate(Node node) =>
 	/// We consider all status codes >= 200 and < 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)
 	{

src/Elastic.Transport/Products/Elasticsearch/Responses/ElasticsearchDynamicResponse.cs

@@ -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);

src/Elastic.Transport/Products/Elasticsearch/Responses/ElasticsearchJsonResponse.cs

@@ -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);

src/Elastic.Transport/Products/Elasticsearch/Responses/ElasticsearchPipeResponse.cs

@@ -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);

src/Elastic.Transport/Products/Elasticsearch/Responses/ElasticsearchResponse.cs

@@ -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]

src/Elastic.Transport/Products/Elasticsearch/Responses/ElasticsearchResponseHelper.cs

@@ -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;
 

src/Elastic.Transport/Products/Elasticsearch/Responses/ElasticsearchStreamResponse.cs

@@ -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);

src/Elastic.Transport/Products/Elasticsearch/Responses/ElasticsearchStringResponse.cs

@@ -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);

src/Elastic.Transport/Products/ProductRegistration.cs

@@ -171,4 +171,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;
 }