Skip to content

Keep safe response consumer when converting transport options - #1355

Merged
l-trotta merged 3 commits into
elastic:mainfrom
RaphaelFakhri:fix-options-safe-consumer
Nov 9, 2026
Merged

l-trotta merged 3 commits into
elastic:mainfrom
RaphaelFakhri:fix-options-safe-consumer

Conversation

@RaphaelFakhri

Copy link
Copy Markdown
Contributor

Problem

RestClientOptions.of() and Rest5ClientOptions.of() convert any other TransportOptions implementation by starting from RequestOptions.DEFAULT. The converted options therefore use the plain response consumer factory instead of SafeResponseConsumer.DEFAULT_FACTORY, which initialOptions() uses to keep errors thrown while consuming a response from shutting down the I/O reactor.

This conversion runs on the default setup path. The ElasticsearchClient.of(...) builder methods transportOptions(fn) and retryConfig(...) produce DefaultTransportOptions, which the transport then converts:

ElasticsearchClient client = ElasticsearchClient.of(b -> b
    .host("https://localhost:9200")
    .apiKey(apiKey)
    .retryConfig(r -> r.maxRetries(3)) // or .transportOptions(o -> o.addHeader(...))
);
// client._transportOptions() no longer uses SafeResponseConsumer

The same happens with client.withTransportOptions(DefaultTransportOptions) or a custom TransportOptions. With either transport, an Error such as OutOfMemoryError raised while reading a response can then stop the I/O reactor and leave the client permanently unusable, the failure mode that SafeResponseConsumer was added to prevent (#1046, #1049).

Change

  • Start the conversion from SafeResponseConsumer.DEFAULT_REQUEST_OPTIONS in both RestClientOptions.of() and Rest5ClientOptions.of(), matching initialOptions().
  • Add a test to RestClientOptionsTest and Rest5ClientOptionsTest that builds a client with transportOptions(...) and checks that the resulting options use SafeResponseConsumer.DEFAULT_FACTORY.

Testing

./gradlew :java-client:test --tests co.elastic.clients.transport.rest_client.RestClientOptionsTest \
  --tests co.elastic.clients.transport.rest5_client.Rest5ClientOptionsTest
  • Without the fix: the 2 new tests fail (12 tests, 2 failed).
  • With the fix: all tests pass, together with SafeResponseConsumerTest (both transports) and ElasticsearchTransportConfigTest.
  • checkstyleMain, checkstyleTest, forbiddenApisMain and forbiddenApisTest pass.

Fixes #1354

RaphaelFakhri and others added 2 commits October 19, 2026 08:37
RestClientOptions.of() and Rest5ClientOptions.of() built converted options
from RequestOptions.DEFAULT, so options set with the client builder's
transportOptions() or retryConfig(), or passed as DefaultTransportOptions,
lost the SafeResponseConsumer that prevents I/O reactor shutdowns. Start
from SafeResponseConsumer.DEFAULT_REQUEST_OPTIONS instead, as
initialOptions() does.
@l-trotta

l-trotta commented Nov 5, 2026

Copy link
Copy Markdown
Contributor

Hello @RaphaelFakhri , thank you for reporting the issue and for the fix! It looks good to be merged, I just have a small nit: instead of calling esClient._transport().close() at the end of the tests, could you use the try with resources block when you create the client? like try (ElasticsearchClient esClient = ...). Thanks again :)

@RaphaelFakhri

Copy link
Copy Markdown
Contributor Author

Thanks, done in e643286. Both tests now create the client in a try-with-resources block.

@l-trotta l-trotta left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

thank you!

@l-trotta
l-trotta merged commit 7321353 into elastic:main Nov 9, 2026
6 of 10 checks passed
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.

Transport options set on the client builder drop SafeResponseConsumer

2 participants