Add Elasticsearch specific response types

[flobernd]

Summary

The existing ElasticsearchResponse provides Elasticsearch-specific error handling (IsValidResponse, ElasticsearchServerError, ElasticsearchWarnings, DebugInformation) but requires JSON-deserialized subclasses. Native response types (StringResponse, StreamResponse, PipeResponse, etc.) have no Elasticsearch error handling at all.

This is a problem for endpoints returning non-JSON responses (ESQL binary, ML models, etc.) — we want StreamResponse-style body access with ElasticsearchResponse-style error handling. The ESQL client also benefits from this.

This PR introduces:

1. IElasticsearchResponse interface — common contract for Elasticsearch error handling, implemented by both the existing ElasticsearchResponse base and new native-type variants.

2. Elasticsearch-flavored response types — ElasticsearchStringResponse, ElasticsearchStreamResponse, ElasticsearchDynamicResponse, ElasticsearchJsonResponse, ElasticsearchPipeResponse. Each extends a shared base class and implements IElasticsearchResponse. Dispose semantics are natural (stream/pipe types inherit IDisposable/IAsyncDisposable from their base).

3. Base classes for all special response types — StringResponseBase, DynamicResponseBase, JsonResponseBase, PipeResponseBase (following the existing StreamResponseBase pattern). Existing types re-parented. Shared behavior lives in the base; both transport-level and Elasticsearch-level types derive from the same base.

4. Generic response builders — StringResponseBuilder<T>, DynamicResponseBuilder<T>, JsonResponseBuilder<T>. Body-building logic written once against the base type. Non-generic aliases kept for backward compat. Zero duplication between transport and Elasticsearch builders.

5. Centralized error extraction in DefaultResponseFactory — product-specific error handling moved from builders into the factory via two new ProductRegistration extension points:

   • IsErrorContentType(string?) — does the content-type indicate a parseable error body? Prevents JSON-parsing HTML proxy errors.
   • TryExtractError(BoundConfiguration, Stream) — extract a product-specific ErrorResponse from a seekable stream.

   The factory stores the result in a new ApiCallDetails.ProductError property. All IElasticsearchResponse types read ElasticsearchServerError as a computed property from ApiCallDetails.ProductError — no stored state, no setter interfaces.

6. Error body always buffered on failure — for non-success status codes, the response body is always captured in ApiCallDetails.ResponseBodyInBytes, even when the content-type doesn't match the product's error format (HTML from proxies, unknown JSON, binary). Users can always inspect the raw error body.

7. Improved DefaultResponseFactory readability — flattened nesting, early returns, extracted BufferResponseStreamAsync/CaptureResponseBytesAsync helpers, proper Stream? nullability annotations, FinalizeResponse() local function.

Key architectural decisions

• Concrete types over generic wrapper (ElasticsearchResponse<T>) — generic wrapper can't express dispose semantics (ElasticsearchResponse<StreamResponse> isn't IDisposable). Concrete types make disposal discoverable to users and analyzers.
• Base classes over wrapping — ElasticsearchStreamResponse : StreamResponseBase rather than ElasticsearchResponse<StreamResponse>. Natural inheritance, no body-copying, no forwarding.
• Factory-level error extraction — builders are purely about body construction. Error handling happens once in the factory, before any builder runs. Eliminates the need for error decorators, setter interfaces, or type-dispatch switches.
• IsErrorContentType gate — error extraction only attempts JSON parsing when the content-type is application/json or application/vnd.elasticsearch+json. Prevents wasted work on proxy HTML errors.
• Computed ElasticsearchServerError — reads from ApiCallDetails.ProductError, no stored property or internal set. No IElasticsearchResponseSetter interface needed.

Breaking changes

• StreamResponseBase constructor changed from public to protected (it's an abstract class).
• Stream responseStream parameter in ResponseFactory.Create/CreateAsync changed to Stream? responseStream (nullable).

[Mpdreamz]

Alternative design: avoid the 7 concrete Elasticsearch*Response types

The PR achieves the right goal but introduces significant boilerplate: 7 near-identical concrete types + 6 new *ResponseBase classes + generic builder variants + an 8-arm SetServerError switch. Net +681 lines for what is conceptually a small addition.

Proposed alternative: single ElasticsearchResponse<TBody>

Replace all 7 concrete response types with one generic:

public sealed class ElasticsearchResponse<TBody> : TransportResponse, IElasticsearchResponse
{
    public TBody? Body { get; internal set; }
    public ElasticsearchServerError? ElasticsearchServerError { get; internal set; }

    public bool IsValidResponse =>
        ElasticsearchResponseHelper.IsValidResponse(ApiCallDetails, ElasticsearchServerError);
    public IEnumerable<string> ElasticsearchWarnings =>
        ElasticsearchResponseHelper.GetElasticsearchWarnings(ApiCallDetails);
    public string DebugInformation =>
        ElasticsearchResponseHelper.GetDebugInformation(IsValidResponse, ApiCallDetails, ElasticsearchServerError);
    public bool TryGetOriginalException(out Exception? exception) =>
        ElasticsearchResponseHelper.TryGetOriginalException(ApiCallDetails, out exception);
    public override string ToString() => DebugInformation;
}

• ElasticsearchResponse<string> replaces ElasticsearchStringResponse
• ElasticsearchResponse<byte[]> replaces ElasticsearchBytesResponse
• ElasticsearchResponse<DynamicDictionary> replaces ElasticsearchDynamicResponse
• ElasticsearchResponse<JsonNode> replaces ElasticsearchJsonResponse
• ElasticsearchResponse<Stream> replaces ElasticsearchStreamResponse
• ElasticsearchResponse<Void> replaces ElasticsearchVoidResponse

Builder: factory delegate, no casts

Instead of a BuildBody that does (TBody)(object) double-casts, inject the deserialization logic as a delegate at construction:

internal sealed class ElasticsearchPrimitiveResponseBuilder<TBody> : IResponseBuilder
{
    private readonly Func<BoundConfiguration, Stream, string, long, TBody?> _factory;

    public ElasticsearchPrimitiveResponseBuilder(
        Func<BoundConfiguration, Stream, string, long, TBody?> factory) => _factory = factory;

    bool IResponseBuilder.CanBuild<TResponse>() => typeof(TResponse) == typeof(ElasticsearchResponse<TBody>);

    TResponse? IResponseBuilder.Build<TResponse>(ApiCallDetails details,
        BoundConfiguration config, Stream stream, string contentType, long contentLength)
    {
        var response = new ElasticsearchResponse<TBody> { Body = _factory(config, stream, contentType, contentLength) };
        return response as TResponse;
    }
}

Registration becomes self-documenting and cast-free:

public override IReadOnlyCollection<IResponseBuilder> ResponseBuilders { get; } =
[
    new ElasticsearchErrorDecorator<ElasticsearchResponse<string>>(
        new ElasticsearchPrimitiveResponseBuilder<string>((cfg, s, _, _) => s.ReadToEnd(cfg.MemoryStreamFactory))),
    new ElasticsearchErrorDecorator<ElasticsearchResponse<byte[]>>(
        new ElasticsearchPrimitiveResponseBuilder<byte[]>((cfg, s, _, _) => s.ToByteArray())),
    new ElasticsearchErrorDecorator<ElasticsearchResponse<DynamicDictionary>>(
        new ElasticsearchPrimitiveResponseBuilder<DynamicDictionary>((cfg, s, _, _) => cfg.RequestResponseSerializer.Deserialize<DynamicDictionary>(s))),
    new ElasticsearchErrorDecorator<ElasticsearchResponse<JsonNode>>(
        new ElasticsearchPrimitiveResponseBuilder<JsonNode>((cfg, s, _, _) => JsonNode.Parse(s))),
    // Stream/Void/Pipe keep their trivial builders (they don't read the stream)
    new ElasticsearchErrorDecorator<ElasticsearchResponse<Stream>>(new ElasticsearchStreamResponseBuilder()),
    new ElasticsearchErrorDecorator<ElasticsearchResponse<Void>>(new ElasticsearchVoidResponseBuilder()),
    new ElasticsearchResponseBuilder()
];

Fix SetServerError — eliminate the 8-arm switch

Add an internal setter interface:

internal interface IElasticsearchResponseSetter
{
    ElasticsearchServerError? ElasticsearchServerError { set; }
}

Both ElasticsearchResponse<TBody> and the existing ElasticsearchResponse abstract base implement it. Then:

// Before (8-arm switch):
switch (response)
{
    case ElasticsearchResponse r: r.ElasticsearchServerError = error; break;
    case ElasticsearchStringResponse r: r.ElasticsearchServerError = error; break;
    // ...6 more arms...
}

// After:
if (response is IElasticsearchResponseSetter r)
    r.ElasticsearchServerError = error;

Preserving Get<T>(path) for dynamic/JSON

DynamicResponseBase.Get<T> and JsonResponseBase.Get<T> are no longer inherited, but they're easily restored via extension methods:

public static T Get<[DynamicallyAccessedMembers(...)] T>(
    this ElasticsearchResponse<DynamicDictionary> r, string path) =>
    r.Body?.Get<T>(path) ?? default!;

public static T Get<[DynamicallyAccessedMembers(...)] T>(
    this ElasticsearchResponse<JsonNode> r, string path) =>
    JsonNodePathTraversal.Get<T>(r.Body, path);

What's eliminated vs this PR

	PR #205	Alternative
New public types	7 (Elasticsearch*Response)	1 (ElasticsearchResponse<T>)
New base classes	6 (*ResponseBase)	0
Generic builder variants	4	0
SetServerError switch arms	8	0 (interface cast)
Net new lines (approx)	+681	~+150

Trade-off to consider

ElasticsearchStringResponse is more discoverable in docs/IntelliSense than ElasticsearchResponse<string>. If the named types are important for the public API surface of elasticsearch-net, that's a valid reason to keep them — but the 6 base classes and the SetServerError switch should still be cleaned up regardless.

[flobernd]

@Mpdreamz
I was considering this exact design, but decided against it mainly to keep correct Dispose semantics and discoverability.

Bodies like StreamResponse are disposable (or IAsyncDisposable for PipeReader).
This means the proposed design must implement IDisposable and IAsyncDisposable for all Elasticsearch specific response types which makes it extremely hard for the users (and analyzers) to know when it really must be disposed.

Besides that:

Builder: factory delegate, no casts

This sounds good in theory, but does not work out (it at least did not for me). In my test implementation had to duplicate a lot of code due to the error detection + stream duplication / reset functionality and the different ownership semantics.

Good suggestion for the internal SetServerError interface!

Regarding the Base classes:

Maybe I should have split this into 2 different PRs. The change to Base requests was not only driven by the need for the new Elasticsearch specific responses, but also to allow "minimal visibility" sub-classing for consumers - We already do this for StreamResponse{Base} and I think it makes sense for other responses as well (at least for PipeResponse, but I can also see this being used for e.g. StringResponse or JsonResponse).

The use case is that consumers sub-class e.g. StreamResponseBase and do custom parsing of the stream while still keeping the transport level stream private (protected).

[Mpdreamz]

Good points! Especially the disposal semantics on a generic type — I can see now why you swayed towards the concrete types. The internal interface is still worth picking up to kill the switch, but the overall design is sound. 👍

[Mpdreamz]

Good points! Especially the disposal semantics on a generic type — I can see now why you swayed towards the concrete types. The IElasticsearchResponseSetter internal interface is still worth picking up to kill the switch, but the overall design is sound.

[flobernd]

Thanks @Mpdreamz , I'll refactor the switch away using your suggestion before I merge. I'll also remove the byte[] and void variants since we don't have a use case for them at the moment.

[flobernd]

I switched back to draft since since I'm currently exploring just another design. I'm off next week, but I'll pick it up again the week after 🙂

[flobernd]

I updated the design (check the new PR description). Could you please have another look @Mpdreamz ?

[Mpdreamz]

Overall this is a solid direction: centralizing extraction, ProductError, and clearer ES-flavored response types makes the pipeline easier to reason about than scattering error handling in builders.

Below are a few spots where behavior changes in ways that can surprise callers or regress diagnostics. If these trade-offs are deliberate, it would help to document them in code (e.g. /// <remarks> on HttpStatusCodeClassifier / IsErrorContentType / TryExtractError, TryGetServerErrorReason, ElasticsearchResponseHelper.IsValidResponse, and/or DefaultResponseFactory around mayHaveErrorBody), so future readers do not assume parity with main without reading the PR.

---

404 and IsValidResponse

On main, every HTTP 404 was treated as an invalid Elasticsearch response. On this PR, 404 can be valid when no extracted server error has HasError(), and invalid when it does.

If intentional: this is a compatibility- and test-sensitive change; worth an explicit remark that 404 is not universally “invalid” and that callers must not rely on status alone for document-not-found vs real failure.

---

TryGetServerErrorReason

On main, a reason could still be produced by parsing StringResponse / BytesResponse / generic TransportResponse via TryGetElasticsearchServerError. Here, reasons come only from ApiCallDetails.ProductError when it is an ElasticsearchServerError with HasError().

If intentional: document that there is no body-parse fallback for reasons, so TryGetServerErrorReason may return false even when ResponseBodyInBytes contains human-readable or parseable content (e.g. after content-type gating or failed extraction).

---

IsErrorContentType + TryExtractError

On main, the builder’s error branch attempted error deserialization without this content-type gate. Here, ProductError is only populated when IsErrorContentType passes.

If intentional: document that structured ProductError depends on headers, and that non-JSON / unexpected Content-Type means no ProductError, with raw diagnostics in ResponseBodyInBytes instead.

---

Non-success body path includes 3xx

On main, the Elasticsearch builder’s “try error first” logic was tied to HttpStatusCode > 399. Here, mayHaveErrorBody follows !HttpStatusCodeClassifier (for Elasticsearch, outside 200–299), so 3xx enters buffering / extraction-related logic, not only 4xx/5xx.

If intentional: a short remark on why 3xx shares this path (or that it is harmless by design) would prevent future “bug” reports about redirects.

---

Short-circuit when ProductError?.HasError()

Skipping full TResponse deserialization when a structured error is recognized is not new in spirit, but it is now factory-driven. If extraction ever misclassifies, callers still lose the full typed body—same risk class as before, different layer.

If intentional: a one-line note that full body deserialize is skipped when ProductError indicates a real error helps set expectations for edge cases.

---

Ask

If the maintainers confirm these behaviors, please encode them in /// remarks (or a single central comment in DefaultResponseFactory) so the PR’s intent stays obvious after merge. Happy to re-review after those docs land.

[Mpdreamz]

The remarks bit is AI slop, I just want to ensure all the behavioral changes are intended :) @flobernd

[flobernd]

Thanks for the review @Mpdreamz

404 and IsValidResponse

This change is intended. This was incorrect earlier as commented here:

// 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.

Cases like GET /my-index/_doc/missing-doc-id return a regular response with "found": false instead of an error JSON.

TryGetServerErrorReason

AI slop as well 😅 There is a fallback that preserves the old semantics.

IsErrorContentType + TryExtractError

Intentional. Makes no sense trying to parse things like http/text/etc from a proxy into a structured error type that expects JSON.

Non-success body path includes 3xx

My thinking was that we usually won't see these codes since HttpClient already handles redirects. For some unhandled cases this change causes them to go to the error path first. They mostly won't have a body, but if so the content will end up in the ResponseBodyAsBytes since error parsing fails.

Short-circuit when ProductError?.HasError()

If that condition is met, we definitely have an error. No need to call custom response builders IMO.

[Mpdreamz]

Thanks again for walking through the behavioral intent — super helpful.

One follow-up on TryGetServerErrorReason and how it relates to TryGetElasticsearchServerError, with concrete code so we’re aligned.

1. On main, TryGetServerErrorReason parses from the body (three paths)

ElasticsearchProductRegistration.cs on main uses StringResponse → BytesResponse → TransportResponse and calls TryGetElasticsearchServerError on each branch. That method ultimately reads Body / ResponseBodyInBytes (see extensions below) when the content-type checks pass.

public override bool TryGetServerErrorReason<TResponse>(TResponse response, out string? reason)
{
	reason = null;
	if (response is StringResponse s && s.TryGetElasticsearchServerError(out var e))
		reason = e?.Error?.ToString();
	else if (response is BytesResponse b && b.TryGetElasticsearchServerError(out e))
		reason = e?.Error?.ToString();
	else if (response.TryGetElasticsearchServerError(out e))
		reason = e?.Error?.ToString();
	return e != null;
}

2. On the PR branch, TryGetServerErrorReason only looks at ProductError

On elasticsearch-response-types, the StringResponse / BytesResponse / TryGetElasticsearchServerError chain is gone. The only source of a reason is ApiCallDetails.ProductError:

public override bool TryGetServerErrorReason<TResponse>(TResponse response, out string? reason)
{
	reason = null;

	if (response.ApiCallDetails?.ProductError is ElasticsearchServerError error && error.HasError())
	{
		reason = error.Error?.ToString();
		return true;
	}

	return false;
}

So if ProductError was never set (e.g. IsErrorContentType false, extraction failed, etc.) but ResponseBodyInBytes still holds JSON that the old extension path could parse, TryGetServerErrorReason returns false — there is no second chance that parses the body inside this method.

3. Where that shows up: RequestPipeline / TransportException message

RequestPipeline.cs on the PR branch only appends ServerError: when TryGetServerErrorReason succeeds:

if (response != null && _productRegistration.TryGetServerErrorReason(response, out var reason))
	exceptionMessage += $". ServerError: {reason}";

So the fallback loss we’re talking about is: exception messages may omit ServerError: … in cases where main would still have appended it via body parsing, because TryGetServerErrorReason no longer calls into TryGetElasticsearchServerError.

4. The extensions are a different API — on the PR they do keep a body fallback

Separately, ElasticsearchErrorExtensions on main only parses from body/bytes (example: TransportResponse overload):

public static bool TryGetElasticsearchServerError(this TransportResponse response, out ElasticsearchServerError? serverError)
{
	serverError = null;
	var bytes = response.ApiCallDetails.ResponseBodyInBytes;
	if (bytes == null || response.ApiCallDetails.ResponseContentType != BoundConfiguration.DefaultContentType)
		return false;

	var settings = response.ApiCallDetails.TransportConfiguration;
	using var stream = settings.MemoryStreamFactory.Create(bytes);
	return ElasticsearchServerError.TryCreate(stream, out serverError);
}

On the PR branch, the same overloads first try ProductError, then fall back to that body parse:

public static bool TryGetElasticsearchServerError(this TransportResponse response, out ElasticsearchServerError? serverError)
{
	// Prefer the factory-extracted error if available
	if (TryGetFactoryExtractedError(response, out serverError))
		return true;

	serverError = null;
	var bytes = response.ApiCallDetails.ResponseBodyInBytes;
	if (bytes == null || response.ApiCallDetails.ResponseContentType != BoundConfiguration.DefaultContentType)
		return false;

	var settings = response.ApiCallDetails.TransportConfiguration;
	using var stream = settings.MemoryStreamFactory.Create(bytes);
	return ElasticsearchServerError.TryCreate(stream, out serverError);
}

private static bool TryGetFactoryExtractedError(TransportResponse response, out ElasticsearchServerError? serverError)
{
	serverError = response.ApiCallDetails?.ProductError as ElasticsearchServerError;
	return serverError?.HasError() ?? false;
}

Important: TryGetServerErrorReason does not call these extension methods on the PR, so this “ProductError first, else body” behavior does not automatically fix the RequestPipeline / TransportException path above.

If we want main-parity for the exception string whenever body parsing would still work, we’d need either an explicit fallback chain inside TryGetServerErrorReason (after ProductError) or a documented guarantee that ProductError is always populated when the old body path would have succeeded.

[flobernd]

Thanks for the thorough walkthrough — really appreciate the side-by-side code traces.

I believe ProductError is always populated when the old body-parsing path would have succeeded, so the fallback chain in TryGetServerErrorReason is intentionally removed rather than accidentally lost. Here's the reasoning:

ProductError extraction is response-type-agnostic

In DefaultResponseFactory, the error extraction path (lines 113–128) is gated only on mayHaveErrorBody && IsErrorContentType(contentType) — it never branches on TResponse. Whether the caller requests a StringResponse, BytesResponse, or any other type, the same extraction runs and populates ApiCallDetails.ProductError before the response object is built.

The new content-type check is more permissive than the old one

On main, the extension methods used an exact equality check:

response.ApiCallDetails.ResponseContentType != BoundConfiguration.DefaultContentType // "application/json"

This rejected application/json; charset=utf-8, application/vnd.elasticsearch+json;compatible-with=8, or any casing variation.

The new IsErrorContentType uses StartsWith with OrdinalIgnoreCase and also covers the vendor content-type — a strict superset of what the old code accepted. So every response where the old fallback would have succeeded, ProductError is now extracted during response creation.

The deserialization is equivalent

Both paths deserialize using the same serializer. The only difference is that TryExtractError gates on HasError() (Status > 0 && Error != null) while the old TryCreate just checked != null. Any well-formed Elasticsearch error response satisfies HasError() — the only things filtered out are malformed responses where error is null or status is 0, which aren't useful to surface anyway.

Non-Elasticsearch registrations are unaffected

TryGetServerErrorReason is abstract on the base ProductRegistration. DefaultProductRegistration overrides it to always return false — same as on main. The Elasticsearch-specific extension methods were never called outside of ElasticsearchProductRegistration, so there's no behavioral change for other registrations.

The extension methods keep their fallback for the public API

The TryGetElasticsearchServerError extensions on the PR branch still try ProductError first and fall back to body parsing — so callers using that API directly still get the fallback behavior. It's only the internal TryGetServerErrorReason path (used by RequestPipeline for exception messages) that relies solely on ProductError, which is justified because the factory guarantees it's populated for all valid Elasticsearch error responses.