Skip to content

Commit 05fc109

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
Conflicts: src/Elasticsearch.Net/Responses/ElasticsearchResponse.cs src/Elasticsearch.Net/Transport/Pipeline/ResponseBuilder.cs src/Tests/Document/Multiple/ReindexOnServer/ReindexOnServerApiTests.cs src/Tests/Document/Multiple/ReindexOnServer/ReindexOnServerRemoteApiTests.cs
1 parent a7a296a commit 05fc109

File tree

7 files changed

+83
-27
lines changed

7 files changed

+83
-27
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
@@ -67,6 +67,8 @@ public class ElasticsearchResponse<T> : IApiCallDetails
6767

6868
public List<Audit> AuditTrail { get; internal set; }
6969

70+
internal bool AllowAllStatusCodes { get; }
71+
7072
/// <summary>
7173
/// The response is successful or has a response code between 400-599, the call should not be retried.
7274
/// Only on 502,503 and 504 will this return false;
@@ -91,7 +93,8 @@ public ElasticsearchResponse(Exception e)
9193
public ElasticsearchResponse(int statusCode, IEnumerable<int> allowedStatusCodes)
9294
{
9395
var statusCodes = allowedStatusCodes as int[] ?? allowedStatusCodes.ToArray();
94-
this.Success = statusCode >= 200 && statusCode < 300 || statusCodes.Contains(statusCode) || statusCodes.Contains(-1);
96+
this.AllowAllStatusCodes = statusCodes.Contains(-1);
97+
this.Success = statusCode >= 200 && statusCode < 300 || statusCodes.Contains(statusCode) || this.AllowAllStatusCodes;
9598
this.HttpStatusCode = statusCode;
9699
}
97100

‎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 System.Threading;
98
using Purify;

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

Lines changed: 29 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ private ElasticsearchResponse<TReturn> Initialize(int? statusCode, Exception exc
5858
private void SetBody(ElasticsearchResponse<TReturn> response, Stream stream)
5959
{
6060
byte[] bytes = null;
61-
if (NeedsToEagerReadStream())
61+
if (NeedsToEagerReadStream(response))
6262
{
6363
var inMemoryStream = this._requestData.MemoryStreamFactory.Create();
6464
stream.CopyTo(inMemoryStream, BufferSize);
@@ -68,29 +68,25 @@ private void SetBody(ElasticsearchResponse<TReturn> response, Stream stream)
6868
var needsDispose = typeof(TReturn) != typeof(Stream);
6969
using (needsDispose ? stream : EmptyDisposable)
7070
{
71-
if (response.Success)
71+
if (response.Success || response.AllowAllStatusCodes)
7272
{
7373
if (!SetSpecialTypes(stream, response, bytes))
7474
{
7575
if (this._requestData.CustomConverter != null) response.Body = this._requestData.CustomConverter(response, stream) as TReturn;
7676
else response.Body = this._requestData.ConnectionSettings.Serializer.Deserialize<TReturn>(stream);
7777
}
78+
if (response.AllowAllStatusCodes)
79+
ReadServerError(response, new MemoryStream(bytes), bytes);
7880
}
7981
else if (response.HttpStatusCode != null)
80-
{
81-
ServerError serverError;
82-
if (ServerError.TryCreate(stream, out serverError))
83-
response.ServerError = serverError;
84-
if (_disableDirectStreaming)
85-
response.ResponseBodyInBytes = bytes;
86-
}
82+
ReadServerError(response, stream, bytes);
8783
}
8884
}
8985

9086
private async Task SetBodyAsync(ElasticsearchResponse<TReturn> response, Stream stream)
9187
{
9288
byte[] bytes = null;
93-
if (NeedsToEagerReadStream())
89+
if (NeedsToEagerReadStream(response))
9490
{
9591
var inMemoryStream = this._requestData.MemoryStreamFactory.Create();
9692
await stream.CopyToAsync(inMemoryStream, BufferSize, this._requestData.CancellationToken).ConfigureAwait(false);
@@ -100,23 +96,37 @@ private async Task SetBodyAsync(ElasticsearchResponse<TReturn> response, Stream
10096
var needsDispose = typeof(TReturn) != typeof(Stream);
10197
using (needsDispose ? stream : EmptyDisposable)
10298
{
103-
if (response.Success)
99+
if (response.Success || response.AllowAllStatusCodes)
104100
{
105101
if (!SetSpecialTypes(stream, response, bytes))
106102
{
107103
if (this._requestData.CustomConverter != null) response.Body = this._requestData.CustomConverter(response, stream) as TReturn;
108104
else response.Body = await this._requestData.ConnectionSettings.Serializer.DeserializeAsync<TReturn>(stream, this._requestData.CancellationToken).ConfigureAwait(false);
109105
}
106+
if (response.AllowAllStatusCodes)
107+
await ReadServerErrorAsync(response, new MemoryStream(bytes), bytes);
110108
}
111109
else if (response.HttpStatusCode != null)
112-
{
113-
response.ServerError = await ServerError.TryCreateAsync(stream, this._requestData.CancellationToken).ConfigureAwait(false);
114-
if (_disableDirectStreaming)
115-
response.ResponseBodyInBytes = bytes;
116-
}
110+
await ReadServerErrorAsync(response, stream, bytes);
117111
}
118112
}
119113

114+
private void ReadServerError(ElasticsearchResponse<TReturn> response, Stream stream, byte[] bytes)
115+
{
116+
ServerError serverError;
117+
if (ServerError.TryCreate(stream, out serverError))
118+
response.ServerError = serverError;
119+
if (_disableDirectStreaming)
120+
response.ResponseBodyInBytes = bytes;
121+
}
122+
123+
private async Task ReadServerErrorAsync(ElasticsearchResponse<TReturn> response, Stream stream, byte[] bytes)
124+
{
125+
response.ServerError = await ServerError.TryCreateAsync(stream, this._cancellationToken).ConfigureAwait(false);
126+
if (_disableDirectStreaming)
127+
response.ResponseBodyInBytes = bytes;
128+
}
129+
120130
private void Finalize(ElasticsearchResponse<TReturn> response)
121131
{
122132
var passAlongConnectionStatus = response.Body as IBodyWithApiCallDetails;
@@ -126,8 +136,9 @@ private void Finalize(ElasticsearchResponse<TReturn> response)
126136
}
127137
}
128138

129-
private bool NeedsToEagerReadStream() =>
130-
_disableDirectStreaming || typeof(TReturn) == typeof(string) || typeof(TReturn) == typeof(byte[]);
139+
private bool NeedsToEagerReadStream(ElasticsearchResponse<TReturn> response) =>
140+
response.AllowAllStatusCodes //need to double read for error and TReturn
141+
|| _disableDirectStreaming || typeof(TReturn) == typeof(string) || typeof(TReturn) == typeof(byte[]);
131142

132143
private byte[] SwapStreams(ref Stream responseStream, ref MemoryStream ms)
133144
{

‎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: 9 additions & 7 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,8 +72,8 @@ protected override LazyResponses ClientUsage() => Calls(
7272
.VersionType(VersionType.Internal)
7373
.Routing(ReindexRouting.Discard)
7474
)
75-
.Script(_script)
76-
.Conflicts(Conflicts.Proceed)
75+
.Script(ss => ss.Inline(PainlessScript).Lang("groovy"))
76+
.Conflicts(Conflicts.Proceed)
7777
.Refresh();
7878

7979
protected override ReindexOnServerRequest Initializer => new ReindexOnServerRequest()
@@ -95,8 +95,8 @@ protected override LazyResponses ClientUsage() => Calls(
9595
VersionType = VersionType.Internal,
9696
Routing = ReindexRouting.Discard
9797
},
98-
Script = new InlineScript(_script),
99-
Conflicts = Conflicts.Proceed,
98+
Script = new InlineScript(PainlessScript) { Lang = "groovy" },
99+
Conflicts = Conflicts.Proceed,
100100
Refresh = true,
101101
};
102102

@@ -127,8 +127,10 @@ protected override void ExpectResponse(IReindexOnServerResponse response)
127127
type = "test",
128128
version_type = "internal"
129129
},
130-
script = new {
131-
inline = _script
130+
script = new
131+
{
132+
inline = this.PainlessScript,
133+
lang = "groovy"
132134
},
133135
source = new {
134136
index = CallIsolatedValue,

‎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+
}

0 commit comments

Comments
 (0)