Skip to content

Commit 744e906

Browse files
aochsneraochsner
committed
Attempting to demonstrate and resolve #711
There is also some test cleanup in RetryTests (in addition to some format cleanup...sorry about that noise)
1 parent 9612928 commit 744e906

File tree

4 files changed

+152
-59
lines changed

4 files changed

+152
-59
lines changed

‎src/Elasticsearch.Net/Connection/Transport.cs‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -182,7 +182,7 @@ private ElasticsearchResponse<T> DoRequest<T>(TransportRequestState<T> requestSt
182182

183183
var uri = CreateUriToPath(baseUri, requestState.Path);
184184
bool seenError = false;
185-
185+
var maxRetries = this.GetMaximumRetries(requestState.RequestConfiguration);
186186
try
187187
{
188188
if (shouldPingHint
@@ -193,7 +193,7 @@ private ElasticsearchResponse<T> DoRequest<T>(TransportRequestState<T> requestSt
193193
this.Ping(baseUri);
194194

195195
var streamResponse = _doRequest(requestState.Method, uri, requestState.PostData, requestState.RequestConfiguration);
196-
if (streamResponse != null && streamResponse.SuccessOrKnownError)
196+
if (streamResponse != null && ((maxRetries == 0 && retried == 0) || streamResponse.SuccessOrKnownError))
197197
{
198198
var error = ThrowOrGetErrorFromStreamResponse(requestState, streamResponse);
199199

@@ -206,7 +206,6 @@ private ElasticsearchResponse<T> DoRequest<T>(TransportRequestState<T> requestSt
206206
}
207207
catch (Exception e)
208208
{
209-
var maxRetries = this.GetMaximumRetries(requestState.RequestConfiguration);
210209
if (maxRetries == 0 && retried == 0)
211210
throw;
212211
seenError = true;
@@ -230,8 +229,8 @@ private void SetErrorDiagnosticsAndPatchSuccess<T>(TransportRequestState<T> requ
230229
typedResponse.OriginalException = new ElasticsearchServerException(error);
231230
}
232231
if (!typedResponse.Success
233-
&& requestState.RequestConfiguration != null
234-
&& requestState.RequestConfiguration.AllowedStatusCodes.HasAny(i => i == streamResponse.HttpStatusCode))
232+
&& requestState.RequestConfiguration != null
233+
&& requestState.RequestConfiguration.AllowedStatusCodes.HasAny(i => i == streamResponse.HttpStatusCode))
235234
{
236235
typedResponse.Success = true;
237236
}
@@ -344,9 +343,14 @@ private Task<ElasticsearchResponse<T>> _doRequestAsyncOrRetry<T>(
344343
{
345344
if (t.IsCanceled)
346345
return null;
346+
var maxRetries = this.GetMaximumRetries(requestState.RequestConfiguration);
347347
if (t.IsFaulted)
348-
return this.RetryRequestAsync<T>(requestState, baseUri, retried, t.Exception);
349-
if (t.Result.SuccessOrKnownError)
348+
{
349+
if (maxRetries == 0 && retried == 0)
350+
throw t.Exception;
351+
return this.RetryRequestAsync<T>(requestState, baseUri, retried, t.Exception);
352+
}
353+
if ((maxRetries == 0 && retried == 0) || t.Result.SuccessOrKnownError)
350354
{
351355
var error = ThrowOrGetErrorFromStreamResponse(requestState, t.Result);
352356
return this.StreamToTypedResponseAsync<T>(t.Result, requestState.DeserializationState)
@@ -446,9 +450,9 @@ private ElasticsearchServerError ThrowOrGetErrorFromStreamResponse<T>(
446450
{
447451
ElasticsearchServerError error = null;
448452
if ((!streamResponse.Success && requestState.RequestConfiguration == null)
449-
|| (!streamResponse.Success
450-
&& requestState.RequestConfiguration != null
451-
&& requestState.RequestConfiguration.AllowedStatusCodes.All(i => i != streamResponse.HttpStatusCode)))
453+
|| (!streamResponse.Success
454+
&& requestState.RequestConfiguration != null
455+
&& requestState.RequestConfiguration.AllowedStatusCodes.All(i => i != streamResponse.HttpStatusCode)))
452456
{
453457

454458
if (streamResponse.Response != null && !this.Settings.KeepRawResponse)

‎src/Tests/Elasticsearch.Net.Tests.Unit/Connection/NoRetryTests.cs‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
using System;
2+
using System.IO;
3+
using System.Threading.Tasks;
4+
using Autofac.Extras.FakeItEasy;
5+
using Elasticsearch.Net.Connection;
6+
using Elasticsearch.Net.Tests.Unit.Stubs;
7+
using FakeItEasy;
8+
using NUnit.Framework;
9+
10+
namespace Elasticsearch.Net.Tests.Unit.Connection
11+
{
12+
[TestFixture]
13+
public class NoRetryTests
14+
{
15+
private static readonly int _retries = 0;
16+
17+
//we do not pass a Uri or IConnectionPool so this config
18+
//defaults to SingleNodeConnectionPool()
19+
private readonly ConnectionConfiguration _connectionConfig = new ConnectionConfiguration()
20+
.MaximumRetries(0);
21+
22+
[Test]
23+
public void ShouldNotRetryWhenMaxRetriesIs0()
24+
{
25+
using (var fake = new AutoFake(callsDoNothing: true))
26+
{
27+
var connectionConfiguration = new ConnectionConfiguration().MaximumRetries(0);
28+
fake.Provide<IConnectionConfigurationValues>(connectionConfiguration);
29+
FakeCalls.ProvideDefaultTransport(fake);
30+
31+
var getCall = FakeCalls.GetSyncCall(fake);
32+
getCall.Returns(FakeResponse.Bad(connectionConfiguration));
33+
34+
var client = fake.Resolve<ElasticsearchClient>();
35+
36+
Assert.DoesNotThrow(() => client.Info());
37+
getCall.MustHaveHappened(Repeated.Exactly.Once);
38+
39+
}
40+
}
41+
42+
[Test]
43+
public void ShouldNotRetryWhenMaxRetriesIs0_Async()
44+
{
45+
using (var fake = new AutoFake(callsDoNothing: true))
46+
{
47+
var connectionConfiguration = new ConnectionConfiguration().MaximumRetries(0);
48+
fake.Provide<IConnectionConfigurationValues>(connectionConfiguration);
49+
FakeCalls.ProvideDefaultTransport(fake);
50+
51+
var getCall = FakeCalls.GetCall(fake);
52+
getCall.Returns(FakeResponse.Bad(connectionConfiguration));
53+
54+
var client = fake.Resolve<ElasticsearchClient>();
55+
56+
Assert.DoesNotThrow(async () => await client.InfoAsync());
57+
getCall.MustHaveHappened(Repeated.Exactly.Once);
58+
59+
}
60+
}
61+
62+
[Test]
63+
public void ThrowsException()
64+
{
65+
using (var fake = new AutoFake(callsDoNothing: true))
66+
{
67+
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
68+
FakeCalls.ProvideDefaultTransport(fake);
69+
70+
var getCall = FakeCalls.GetSyncCall(fake);
71+
getCall.Throws<Exception>();
72+
73+
var client = fake.Resolve<ElasticsearchClient>();
74+
75+
Assert.Throws<Exception>(() => client.Info());
76+
getCall.MustHaveHappened(Repeated.Exactly.Once);
77+
78+
}
79+
}
80+
81+
[Test]
82+
public void ThrowsException_Async()
83+
{
84+
using (var fake = new AutoFake(callsDoNothing: true))
85+
{
86+
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
87+
FakeCalls.ProvideDefaultTransport(fake);
88+
var getCall = FakeCalls.GetCall(fake);
89+
90+
//return a started task that throws
91+
Func<ElasticsearchResponse<Stream>> badTask = () => { throw new Exception(); };
92+
var t = new Task<ElasticsearchResponse<Stream>>(badTask);
93+
t.Start();
94+
getCall.Returns(t);
95+
96+
var client = fake.Resolve<ElasticsearchClient>();
97+
98+
Assert.Throws<Exception>(async () => await client.InfoAsync());
99+
getCall.MustHaveHappened(Repeated.Exactly.Once);
100+
101+
}
102+
}
103+
}
104+
}

‎src/Tests/Elasticsearch.Net.Tests.Unit/Connection/RetryTests.cs‎

Lines changed: 33 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,9 @@
11
using System;
2-
using System.Collections.Generic;
32
using System.IO;
4-
using System.Linq;
5-
using System.Text;
63
using System.Threading.Tasks;
7-
using Autofac;
84
using Autofac.Extras.FakeItEasy;
95
using Elasticsearch.Net.Connection;
106
using Elasticsearch.Net.Exceptions;
11-
using Elasticsearch.Net.Providers;
127
using Elasticsearch.Net.Tests.Unit.Stubs;
138
using FakeItEasy;
149
using FluentAssertions;
@@ -36,45 +31,38 @@ public void ThrowsOutOfNodesException_AndRetriesTheSpecifiedTimes()
3631

3732
var getCall = FakeCalls.GetSyncCall(fake);
3833
getCall.Throws<Exception>();
39-
34+
4035
var client = fake.Resolve<ElasticsearchClient>();
4136

4237
client.Settings.MaxRetries.Should().Be(_retries);
4338

44-
Assert.Throws<MaxRetryException>(()=> client.Info());
39+
Assert.Throws<MaxRetryException>(() => client.Info());
4540
getCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
4641

4742
}
4843
}
49-
44+
5045
[Test]
51-
public async void ThrowsOutOfNodesException_AndRetriesTheSpecifiedTimes_Async()
46+
public void ThrowsOutOfNodesException_AndRetriesTheSpecifiedTimes_Async()
5247
{
5348
using (var fake = new AutoFake(callsDoNothing: true))
5449
{
5550
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
5651
FakeCalls.ProvideDefaultTransport(fake);
57-
var getCall = FakeCalls.GetCall(fake);
52+
var getCall = FakeCalls.GetCall(fake);
5853

5954
//return a started task that throws
6055
Func<ElasticsearchResponse<Stream>> badTask = () => { throw new Exception(); };
6156
var t = new Task<ElasticsearchResponse<Stream>>(badTask);
6257
t.Start();
6358
getCall.Returns(t);
64-
59+
6560
var client = fake.Resolve<ElasticsearchClient>();
6661

6762
client.Settings.MaxRetries.Should().Be(_retries);
68-
try
69-
{
70-
var result = await client.InfoAsync();
71-
}
72-
catch (MaxRetryException e)
73-
{
74-
Assert.AreEqual(typeof(MaxRetryException), e.GetType());
75-
}
76-
getCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
7763

64+
Assert.Throws<MaxRetryException>(async () => await client.InfoAsync());
65+
getCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
7866
}
7967
}
8068

@@ -86,35 +74,37 @@ public void ShouldNotRetryOn400()
8674
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
8775
FakeCalls.ProvideDefaultTransport(fake);
8876

89-
var getCall = FakeCalls.GetSyncCall(fake);
77+
var getCall = FakeCalls.GetSyncCall(fake);
9078
getCall.Returns(FakeResponse.Any(_connectionConfig, 400));
91-
79+
9280
var client = fake.Resolve<ElasticsearchClient>();
9381

94-
Assert.DoesNotThrow(()=> client.Info());
82+
Assert.DoesNotThrow(() => client.Info());
9583
getCall.MustHaveHappened(Repeated.Exactly.Once);
9684

9785
}
9886
}
87+
9988
[Test]
10089
public async void ShouldNotRetryOn400_Async()
10190
{
10291
using (var fake = new AutoFake(callsDoNothing: true))
10392
{
10493
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
10594
FakeCalls.ProvideDefaultTransport(fake);
106-
95+
10796
var getCall = FakeCalls.GetCall(fake);
10897
var task = Task.FromResult(FakeResponse.Any(_connectionConfig, 400));
10998
getCall.Returns(task);
110-
99+
111100
var client = fake.Resolve<ElasticsearchClient>();
112101

113102
var result = await client.InfoAsync();
114103
getCall.MustHaveHappened(Repeated.Exactly.Once);
115104

116105
}
117106
}
107+
118108
[Test]
119109
public void ShouldNotRetryOn500()
120110
{
@@ -124,77 +114,71 @@ public void ShouldNotRetryOn500()
124114
FakeCalls.ProvideDefaultTransport(fake);
125115

126116
var getCall = FakeCalls.GetSyncCall(fake);
127-
getCall.Returns(FakeResponse.Any(_connectionConfig, 400));
128-
117+
getCall.Returns(FakeResponse.Any(_connectionConfig, 500));
118+
129119
var client = fake.Resolve<ElasticsearchClient>();
130120

131-
Assert.DoesNotThrow(()=> client.Info());
121+
Assert.DoesNotThrow(() => client.Info());
132122
getCall.MustHaveHappened(Repeated.Exactly.Once);
133123

134124
}
135125
}
136-
126+
137127
[Test]
138128
public void ShouldNotRetryOn201()
139129
{
140130
using (var fake = new AutoFake(callsDoNothing: true))
141131
{
142132
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
143133
FakeCalls.ProvideDefaultTransport(fake);
144-
134+
145135
var getCall = FakeCalls.GetSyncCall(fake);
146136
getCall.Returns(FakeResponse.Any(_connectionConfig, 201));
147-
137+
148138
var client = fake.Resolve<ElasticsearchClient>();
149139

150-
Assert.DoesNotThrow(()=> client.Info());
140+
Assert.DoesNotThrow(() => client.Info());
151141
getCall.MustHaveHappened(Repeated.Exactly.Once);
152142

153143
}
154144
}
155-
145+
156146
[Test]
157147
public void ShouldRetryOn503()
158148
{
159149
using (var fake = new AutoFake(callsDoNothing: true))
160150
{
161151
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
162152
FakeCalls.ProvideDefaultTransport(fake);
163-
153+
164154
var getCall = FakeCalls.GetSyncCall(fake);
165155
getCall.Returns(FakeResponse.Bad(_connectionConfig));
166-
156+
167157
var client = fake.Resolve<ElasticsearchClient>();
168158

169-
Assert.Throws<MaxRetryException>(()=> client.Info());
159+
Assert.Throws<MaxRetryException>(() => client.Info());
170160
getCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
171161

172162
}
173163
}
174-
164+
175165
[Test]
176-
public async void ShouldRetryOn503_Async()
166+
public void ShouldRetryOn503_Async()
177167
{
178168
using (var fake = new AutoFake(callsDoNothing: true))
179169
{
180170
fake.Provide<IConnectionConfigurationValues>(_connectionConfig);
181171
FakeCalls.ProvideDefaultTransport(fake);
182-
172+
183173
var getCall = FakeCalls.GetCall(fake);
184174
getCall.Returns(Task.FromResult(FakeResponse.Bad(_connectionConfig)));
185-
175+
186176
var client = fake.Resolve<ElasticsearchClient>();
187-
try
188-
{
189-
var result = await client.InfoAsync();
190-
}
191-
catch (MaxRetryException e)
192-
{
193-
Assert.AreEqual(e.GetType(), typeof(MaxRetryException));
194-
}
195-
getCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
196177

178+
Assert.Throws<MaxRetryException>(async () => await client.InfoAsync());
179+
getCall.MustHaveHappened(Repeated.Exactly.Times(_retries + 1));
197180
}
198181
}
182+
199183
}
200184
}

‎src/Tests/Elasticsearch.Net.Tests.Unit/Elasticsearch.Net.Tests.Unit.csproj‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@
5858
<Reference Include="System.Xml" />
5959
</ItemGroup>
6060
<ItemGroup>
61+
<Compile Include="Connection\NoRetryTests.cs" />
6162
<Compile Include="Stubs\FakeCalls.cs" />
6263
<Compile Include="Stubs\FakeResponse.cs" />
6364
<Compile Include="Connection\SkipDeadNodesTests.cs" />

0 commit comments

Comments
 (0)