Skip to content

Fix JsonResponseBuilder treating application/vnd.elasticsearch+json as non-JSON#210

Merged
Mpdreamz merged 2 commits into
mainfrom
fix/json-mime-type
May 11, 2026
Merged

Fix JsonResponseBuilder treating application/vnd.elasticsearch+json as non-JSON#210
Mpdreamz merged 2 commits into
mainfrom
fix/json-mime-type

Conversation

@Mpdreamz

Copy link
Copy Markdown
Member

Summary

  • JsonResponseBuilder and DynamicResponseBuilder both gated JSON parsing on contentType.StartsWith("application/json"). When Elasticsearch returns application/vnd.elasticsearch+json (its default when the client sends a matching Accept header), that check fails and the body is wrapped as a raw string under {"body": "..."} instead of being parsed as a JsonNode.
  • This broke ServerReindex.StartReindexAsync in Elastic.Ingest.Elasticsearch: val.Get<string>("task") returned null → "Reindex response did not contain a 'task' field."

Changes

  • ProductRegistration: new virtual IsJsonContentType(string?) — defaults to application/json prefix match.
  • ElasticsearchProductRegistration: overrides to also accept application/vnd.elasticsearch+json (and ;compatible-with=N variants); +x-ndjson and binary subtypes remain unaffected.
  • JsonResponseBuilder / DynamicResponseBuilder: replace hardcoded StartsWith("application/json") with boundConfiguration.ConnectionSettings.ProductRegistration.IsJsonContentType(contentType).
  • JsonResponseTests: two new theory tests — one confirming vendor JSON types are parsed, one confirming non-JSON vendor types still fall through to raw-string mode.

Test plan

  • BuilderElasticsearchVendorJsonContentTypeapplication/vnd.elasticsearch+json (bare, ;compatible-with=8, with space) all parse the JSON body correctly
  • BuilderElasticsearchVendorNonJsonContentType+x-ndjson and +vnd.mapbox-vector-tile still wrap as raw string under body
  • All existing tests pass (205/205 on net10.0, 202/203 on net481 — the one net481 failure is pre-existing and unrelated)

🤖 Generated with Claude Code

…s non-JSON (#210)

Both `JsonResponseBuilder` and `DynamicResponseBuilder` gated JSON parsing on
`contentType.StartsWith("application/json")`. When Elasticsearch returns
`application/vnd.elasticsearch+json` (its default when the client sends a
matching `Accept`), the guard failed and the body was wrapped as a raw string
under a `body` key instead of being parsed as a `JsonNode`. This caused
`val.Get<string>("task")` in `ServerReindex.StartReindexAsync` to return `null`
and throw "Reindex response did not contain a 'task' field."

Fix: add a virtual `ProductRegistration.IsJsonContentType(string?)` that defaults
to `application/json`; `ElasticsearchProductRegistration` overrides it to also
accept `application/vnd.elasticsearch+json` (and `;compatible-with=N` variants)
while leaving `+x-ndjson` and binary subtypes untouched. Both builders now
delegate to `boundConfiguration.ConnectionSettings.ProductRegistration.IsJsonContentType`
instead of the hardcoded prefix check.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@Mpdreamz Mpdreamz requested a review from flobernd May 11, 2026 13:50

@flobernd flobernd left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Maybe we can also redirect this method

public override bool IsErrorContentType(string? contentType) =>
to the new IsJsonContentType.

Eliminates the duplicated application/json + application/vnd.elasticsearch+json
check now that IsJsonContentType encapsulates both cases.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@Mpdreamz Mpdreamz merged commit 0f069d1 into main May 11, 2026
7 checks passed
@Mpdreamz Mpdreamz deleted the fix/json-mime-type branch May 11, 2026 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants