Skip to content

Commit 0285c46

Browse files
MpdreamzMpdreamz
committed
Do a double read for error for response that are always valid since they return valid object shapes on non 200-300 such as reindex and deletebyquery
1 parent 1d9dca4 commit 0285c46

File tree

8 files changed

+79
-25
lines changed

8 files changed

+79
-25
lines changed

‎build/Clients.Common.targets‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
<CurrentVersion>0.0.0-bad</CurrentVersion>
66
<CurrentAssemblyVersion>0.0.0</CurrentAssemblyVersion>
77
<CurrentAssemblyFileVersion>0.0.0.0</CurrentAssemblyFileVersion>
8+
<DotNetCoreOnly></DotNetCoreOnly>
9+
<DoSourceLink></DoSourceLink>
810

911
<!-- Version and Informational reflect actual version -->
1012
<Version>$(CurrentVersion)</Version>

‎src/Elasticsearch.Net/Responses/ElasticsearchResponse.cs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,6 +75,8 @@ public class ElasticsearchResponse<T> : IApiCallDetails
7575

7676
public IEnumerable<string> DeprecationWarnings { get; internal set; } = Enumerable.Empty<string>();
7777

78+
internal bool AllowAllStatusCodes { get; }
79+
7880
/// <summary>
7981
/// The response is successful or has a response code between 400-599, the call should not be retried.
8082
/// Only on 502,503 and 504 will this return false;
@@ -99,7 +101,8 @@ public ElasticsearchResponse(Exception e)
99101
public ElasticsearchResponse(int statusCode, IEnumerable<int> allowedStatusCodes)
100102
{
101103
var statusCodes = allowedStatusCodes as int[] ?? allowedStatusCodes.ToArray();
102-
this.Success = statusCode >= 200 && statusCode < 300 || statusCodes.Contains(statusCode) || statusCodes.Contains(-1);
104+
this.AllowAllStatusCodes = statusCodes.Contains(-1);
105+
this.Success = statusCode >= 200 && statusCode < 300 || statusCodes.Contains(statusCode) || this.AllowAllStatusCodes;
103106
this.HttpStatusCode = statusCode;
104107
}
105108

‎src/Elasticsearch.Net/Transport/Pipeline/RequestData.cs‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
using System.Collections.Specialized;
44
using System.IO;
55
using System.Linq;
6-
using System.Net.Security;
76
using System.Security.Cryptography.X509Certificates;
87
using Purify;
98

‎src/Elasticsearch.Net/Transport/Pipeline/ResponseBuilder.cs‎

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ private ElasticsearchResponse<TReturn> Initialize(int? statusCode, Exception exc
6666
private void SetBody(ElasticsearchResponse<TReturn> response, Stream stream)
6767
{
6868
byte[] bytes = null;
69-
if (NeedsToEagerReadStream())
69+
if (NeedsToEagerReadStream(response))
7070
{
7171
var inMemoryStream = this._requestData.MemoryStreamFactory.Create();
7272
stream.CopyTo(inMemoryStream, BufferSize);
@@ -76,29 +76,25 @@ private void SetBody(ElasticsearchResponse<TReturn> response, Stream stream)
7676
var needsDispose = typeof(TReturn) != typeof(Stream);
7777
using (needsDispose ? stream : EmptyDisposable)
7878
{
79-
if (response.Success)
79+
if (response.Success || response.AllowAllStatusCodes)
8080
{
8181
if (!SetSpecialTypes(stream, response, bytes))
8282
{
8383
if (this._requestData.CustomConverter != null) response.Body = this._requestData.CustomConverter(response, stream) as TReturn;
8484
else response.Body = this._requestData.ConnectionSettings.Serializer.Deserialize<TReturn>(stream);
8585
}
86+
if (response.AllowAllStatusCodes)
87+
ReadServerError(response, new MemoryStream(bytes), bytes);
8688
}
8789
else if (response.HttpStatusCode != null)
88-
{
89-
ServerError serverError;
90-
if (ServerError.TryCreate(stream, out serverError))
91-
response.ServerError = serverError;
92-
if (_disableDirectStreaming)
93-
response.ResponseBodyInBytes = bytes;
94-
}
90+
ReadServerError(response, stream, bytes);
9591
}
9692
}
9793

9894
private async Task SetBodyAsync(ElasticsearchResponse<TReturn> response, Stream stream)
9995
{
10096
byte[] bytes = null;
101-
if (NeedsToEagerReadStream())
97+
if (NeedsToEagerReadStream(response))
10298
{
10399
var inMemoryStream = this._requestData.MemoryStreamFactory.Create();
104100
await stream.CopyToAsync(inMemoryStream, BufferSize, this._cancellationToken).ConfigureAwait(false);
@@ -108,23 +104,37 @@ private async Task SetBodyAsync(ElasticsearchResponse<TReturn> response, Stream
108104
var needsDispose = typeof(TReturn) != typeof(Stream);
109105
using (needsDispose ? stream : EmptyDisposable)
110106
{
111-
if (response.Success)
107+
if (response.Success || response.AllowAllStatusCodes)
112108
{
113109
if (!SetSpecialTypes(stream, response, bytes))
114110
{
115111
if (this._requestData.CustomConverter != null) response.Body = this._requestData.CustomConverter(response, stream) as TReturn;
116112
else response.Body = await this._requestData.ConnectionSettings.Serializer.DeserializeAsync<TReturn>(stream, this._cancellationToken).ConfigureAwait(false);
117113
}
114+
if (response.AllowAllStatusCodes)
115+
await ReadServerErrorAsync(response, new MemoryStream(bytes), bytes);
118116
}
119117
else if (response.HttpStatusCode != null)
120-
{
121-
response.ServerError = await ServerError.TryCreateAsync(stream, this._cancellationToken).ConfigureAwait(false);
122-
if (_disableDirectStreaming)
123-
response.ResponseBodyInBytes = bytes;
124-
}
118+
await ReadServerErrorAsync(response, stream, bytes);
125119
}
126120
}
127121

122+
private void ReadServerError(ElasticsearchResponse<TReturn> response, Stream stream, byte[] bytes)
123+
{
124+
ServerError serverError;
125+
if (ServerError.TryCreate(stream, out serverError))
126+
response.ServerError = serverError;
127+
if (_disableDirectStreaming)
128+
response.ResponseBodyInBytes = bytes;
129+
}
130+
131+
private async Task ReadServerErrorAsync(ElasticsearchResponse<TReturn> response, Stream stream, byte[] bytes)
132+
{
133+
response.ServerError = await ServerError.TryCreateAsync(stream, this._cancellationToken).ConfigureAwait(false);
134+
if (_disableDirectStreaming)
135+
response.ResponseBodyInBytes = bytes;
136+
}
137+
128138
private void Finalize(ElasticsearchResponse<TReturn> response)
129139
{
130140
var passAlongConnectionStatus = response.Body as IBodyWithApiCallDetails;
@@ -134,8 +144,9 @@ private void Finalize(ElasticsearchResponse<TReturn> response)
134144
}
135145
}
136146

137-
private bool NeedsToEagerReadStream() =>
138-
_disableDirectStreaming || typeof(TReturn) == typeof(string) || typeof(TReturn) == typeof(byte[]);
147+
private bool NeedsToEagerReadStream(ElasticsearchResponse<TReturn> response) =>
148+
response.AllowAllStatusCodes //need to double read for error and TReturn
149+
|| _disableDirectStreaming || typeof(TReturn) == typeof(string) || typeof(TReturn) == typeof(byte[]);
139150

140151
private byte[] SwapStreams(ref Stream responseStream, ref MemoryStream ms)
141152
{

‎src/Nest/Document/Multiple/ReindexOnServer/ReindexOnServerResponse.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
using System.Linq.Expressions;
44
using Newtonsoft.Json;
55
using System.Linq;
6+
using Elasticsearch.Net;
67

78
namespace Nest
89
{

‎src/Tests/Document/Multiple/ReindexOnServer/ReindexOnServerApiTests.cs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ protected override LazyResponses ClientUsage() => Calls(
4848

4949
protected override bool SupportsDeserialization => false;
5050

51-
private static string _script = "if (ctx._source.flag == 'bar') {ctx._source.remove('flag')}";
51+
protected virtual string PainlessScript { get; } = "if (ctx._source.flag == 'bar') {ctx._source.remove('flag')}";
5252

5353
protected override Func<ReindexOnServerDescriptor, IReindexOnServerRequest> Fluent => d => d
5454
.Source(s => s
@@ -72,7 +72,7 @@ protected override LazyResponses ClientUsage() => Calls(
7272
.VersionType(VersionType.Internal)
7373
.Routing(ReindexRouting.Discard)
7474
)
75-
.Script(ss => ss.Inline(_script).Lang("groovy"))
75+
.Script(ss => ss.Inline(PainlessScript).Lang("groovy"))
7676
.Conflicts(Conflicts.Proceed)
7777
.Refresh();
7878

@@ -95,7 +95,7 @@ protected override LazyResponses ClientUsage() => Calls(
9595
VersionType = VersionType.Internal,
9696
Routing = ReindexRouting.Discard
9797
},
98-
Script = new InlineScript(_script) { Lang = "groovy" },
98+
Script = new InlineScript(PainlessScript) { Lang = "groovy" },
9999
Conflicts = Conflicts.Proceed,
100100
Refresh = true,
101101
};
@@ -131,7 +131,7 @@ protected override void ExpectResponse(IReindexOnServerResponse response)
131131
},
132132
script = new
133133
{
134-
inline = _script,
134+
inline = this.PainlessScript,
135135
lang = "groovy"
136136
},
137137
source = new

‎src/Tests/Document/Multiple/ReindexOnServer/ReindexOnServerInvalidApiTests.cs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Linq;
4+
using Elasticsearch.Net;
5+
using FluentAssertions;
6+
using Nest;
7+
using Tests.Framework;
8+
using Tests.Framework.Integration;
9+
using Tests.Framework.ManagedElasticsearch.Clusters;
10+
using Tests.Framework.MockData;
11+
using Xunit;
12+
using static Nest.Infer;
13+
14+
namespace Tests.Document.Multiple.ReindexOnServer
15+
{
16+
[SkipVersion("<2.3.0", "")]
17+
public class ReindexOnServerInvalidApiTests : ReindexOnServerApiTests
18+
{
19+
public ReindexOnServerInvalidApiTests(IntrusiveOperationCluster cluster, EndpointUsage usage) : base(cluster, usage) { }
20+
21+
protected override bool ExpectIsValid => false;
22+
protected override int ExpectStatusCode => 500;
23+
24+
//bad painless script
25+
protected override string PainlessScript { get; } = "if ctx._source.flag == 'bar') {ctx._source.remove('flag')}";
26+
27+
protected override void ExpectResponse(IReindexOnServerResponse response)
28+
{
29+
response.ServerError.Should().NotBeNull();
30+
response.ServerError.Status.Should().Be(500);
31+
response.ServerError.Error.Should().NotBeNull();
32+
response.ServerError.Error.RootCause.Should().NotBeNullOrEmpty();
33+
response.ServerError.Error.RootCause.First().Reason.Should().Contain("Error compiling");
34+
response.ServerError.Error.RootCause.First().Type.Should().Be("script_exception");
35+
}
36+
37+
}
38+
}

‎src/Tests/Document/Multiple/ReindexOnServer/ReindexOnServerRemoteApiTests.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
namespace Tests.Document.Multiple.ReindexOnServer
1616
{
1717
[SkipVersion("<2.3.0", "")]
18-
public class ReindexOnServerRemoteApiTests : ApiTestBase<ReadOnlyCluster, IReindexOnServerResponse, IReindexOnServerRequest, ReindexOnServerDescriptor, ReindexOnServerRequest>
18+
public class ReindexOnServerRemoteApiTests : ApiTestBase<IntrusiveOperationCluster, IReindexOnServerResponse, IReindexOnServerRequest, ReindexOnServerDescriptor, ReindexOnServerRequest>
1919
{
2020
private readonly Uri _host = new Uri("http://myremoteserver.example:9200");
2121
public ReindexOnServerRemoteApiTests(IntrusiveOperationCluster cluster, EndpointUsage usage) : base(cluster, usage) { }

0 commit comments

Comments
 (0)