elastic / elasticsearch-net

This strongly-typed, client library enables working with Elasticsearch. It is the official client maintained and supported by Elastic.
https://www.elastic.co/guide/en/elasticsearch/client/net-api/current/index.html
Apache License 2.0
17 stars 1.15k forks source link

7.0 Beta1 - await client.GetIndicesPointingToAliasAsync("non-existent-alias") throws exception #3829

Closed niemyjski closed 5 years ago

niemyjski commented 5 years ago

Using latest stable elastic and 7.0 beta nest client calling await client.GetIndicesPointingToAliasAsync("non-existent-alias") will throw an exception instead of returning an empty collection.

            var indexes = await _client.GetIndicesPointingToAliasAsync(_configuration.DailyLogEvents.Name);
            Assert.Empty(indexes);

            var alias = await _client.Indices.GetAliasAsync(_configuration.DailyLogEvents.Name);
            _logger.LogTraceRequest(alias);
            Assert.False(alias.IsValid);
Elasticsearch.Net.ElasticsearchClientException: Request failed to execute. Call: Status code 404 from: GET /_all/_alias/daily-logevents?pretty=true. ServerError: Type:  Reason: "alias [daily-logevents] missing"
   at Elasticsearch.Net.Transport`1.HandleElasticsearchClientException(RequestData data, Exception clientException, IElasticsearchResponse response)
   at Elasticsearch.Net.Transport`1.FinalizeResponse[TResponse](RequestData requestData, IRequestPipeline pipeline, List`1 seenExceptions, TResponse response)
   at Elasticsearch.Net.Transport`1.RequestAsync[TResponse](HttpMethod method, String path, CancellationToken cancellationToken, PostData data, IRequestParameters requestParameters)
   at Nest.IndicesPointingToAliasExtensions.GetIndicesPointingToAliasAsync(IElasticClient client, Names alias)
   at Foundatio.Repositories.Elasticsearch.Tests.IndexTests.GetByDateBasedIndexAsync() in /Users/blake/Code/Foundatio.Repositories/tests/Foundatio.Repositories.Elasticsearch.Tests/IndexTests.cs:line 111

Looking at the code it seems like it should be returning an empty collection (something that I think we were expecting in the old release).

private static IReadOnlyCollection<string> IndicesPointingToAlias(
      IConnectionSettingsValues settings,
      IUrlParameter alias,
      GetAliasResponse aliasesResponse)
    {
      if (!aliasesResponse.IsValid || !aliasesResponse.Indices.HasAny<KeyValuePair<IndexName, IndexAliases>>())
        return EmptyReadOnly<string>.Collection;
codebrain commented 5 years ago

Hi @niemyjski,

The GetIndicesPointingToAliasAsync is an extension method as defined here:

https://github.com/elastic/elasticsearch-net/blob/602ce089e000ad58c28e41094a0b7394d430fc4e/src/Nest/Indices/AliasManagement/GetAlias/ElasticClient-GetIndicesPointingToAlias.cs#L31

This extension method will throw exceptions (r => r.ThrowExceptions()) if the server returns a 404 - as is the case with a missing alias.

If you want finer control as to throwing exceptions and handling empty collections I would recommend using the client.Indices.GetAliasAsync method on the client directly.

The GetAliasNotFoundApiTests test illustrates the behaviour of this method in the event of a 404 from a missing alias:

https://github.com/elastic/elasticsearch-net/blob/d721d2e3df1f86a808b2a37fef14fe151d4ede7d/src/Tests/Tests/Indices/AliasManagement/GetAlias/GetAliasApiTests.cs#L83-L116

Hopefully this clarifies the behaviour!

Mpdreamz commented 5 years ago

I think @niemyjski opened this because of a possible change in behaviour moving from 6.x

niemyjski commented 5 years ago

Yeah, I opened it because it was a change from 5.x, also go to definition doesn't even show it as an extension for that (that might be bug in rider), it literally pointed me to the code above (weird)

codebrain commented 5 years ago

Relates: https://github.com/elastic/elasticsearch-net/issues/3707

Mpdreamz commented 5 years ago

This should be fixed once #3873 lands

niemyjski commented 5 years ago

Thanks for your help, any eta for the beta release?

codebrain commented 5 years ago

Closing as #3873 has been merged, this should therefore be fixed in the next 7.0.0 release. We are going to be releasing the GA client very shortly.

Please keep an eye on the https://github.com/elastic/elasticsearch-net/releases

Thanks @niemyjski