Skip to content

Commit 2224bef

Browse files
MpdreamzMpdreamz
committed
Issue #2052 showed that BadResponse can be ambiguous
If the exception happens during request serialization, This commit introduces a BadRequest pipelinefailure and audittrail event and improves DebugInformation Improves the exception message by introducing a clear separation between `BadResponse` and a new stage in the pipeline `BadRequest`. If the pipeline failure reason is `Unexpected` we attempt to fetch a more useful failure reason from the audit trail. Another subtle change is that the `DebugInformation` printed a step number that is 0 based. Since this a message intendend for humans and humans read ordinals starting from 1. This now starts at 1 and we print the step number in the audit trail too. New `DebugInformation`: ``` System.Exception: # FailureReason: Unrecoverable/Unexpected BadRequest while attempting POST http://localhost:9200/_bulk - [1] PingSuccess: Node: http://localhost:9200/ Took: 00:00:00.0156239 - [2] BadRequest: Node: http://localhost:9200/ Exception: InvalidOperationException Took: 00:00:00.0997982 System.InvalidOperationException: Method may only be called on a Type for which Type.IsGenericParameter is true. ... ... ... ```
1 parent 398356a commit 2224bef

File tree

13 files changed

+96
-31
lines changed

13 files changed

+96
-31
lines changed

‎src/Elasticsearch.Net/Auditing/AuditEvent.cs‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ public enum AuditEvent
1717
HealthyResponse,
1818

1919
MaxTimeoutReached,
20-
MaxRetriesReached
20+
MaxRetriesReached,
21+
BadRequest
2122
}
22-
}
23+
}

‎src/Elasticsearch.Net/Connection/HttpConnection-CoreFx.cs‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ public virtual ElasticsearchResponse<TReturn> Request<TReturn>(RequestData reque
8383
{
8484
var requestMessage = CreateHttpRequestMessage(requestData);
8585
var response = client.SendAsync(requestMessage, requestData.CancellationToken).GetAwaiter().GetResult();
86+
requestData.MadeItToResponse = true;
8687
builder.StatusCode = (int)response.StatusCode;
8788

8889
if (response.Content != null)
@@ -104,6 +105,7 @@ public virtual async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(
104105
{
105106
var requestMessage = CreateHttpRequestMessage(requestData);
106107
var response = await client.SendAsync(requestMessage, requestData.CancellationToken).ConfigureAwait(false);
108+
requestData.MadeItToResponse = true;
107109
builder.StatusCode = (int)response.StatusCode;
108110

109111
if (response.Content != null)

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,7 @@ public virtual ElasticsearchResponse<TReturn> Request<TReturn>(RequestData reque
132132
data.Write(stream, requestData.ConnectionSettings);
133133
}
134134
}
135+
requestData.MadeItToResponse = true;
135136

136137
//http://msdn.microsoft.com/en-us/library/system.net.httpwebresponse.getresponsestream.aspx
137138
//Either the stream or the response object needs to be closed but not both although it won't
@@ -168,6 +169,7 @@ public virtual async Task<ElasticsearchResponse<TReturn>> RequestAsync<TReturn>(
168169
await data.WriteAsync(stream, requestData.ConnectionSettings).ConfigureAwait(false);
169170
}
170171
}
172+
requestData.MadeItToResponse = true;
171173

172174
//http://msdn.microsoft.com/en-us/library/system.net.httpwebresponse.getresponsestream.aspx
173175
//Either the stream or the response object needs to be closed but not both although it won't

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
using System;
2-
using System.Collections.Generic;
32
using System.IO;
43
using System.IO.Compression;
5-
using System.Text;
64
using System.Threading.Tasks;
75

86
namespace Elasticsearch.Net
@@ -47,6 +45,7 @@ protected ElasticsearchResponse<TReturn> ReturnConnectionStatus<TReturn>(Request
4745
data.Write(stream, requestData.ConnectionSettings);
4846
}
4947
}
48+
requestData.MadeItToResponse = true;
5049

5150
var builder = new ResponseBuilder<TReturn>(requestData)
5251
{
@@ -74,6 +73,7 @@ protected async Task<ElasticsearchResponse<TReturn>> ReturnConnectionStatusAsync
7473
await data.WriteAsync(stream, requestData.ConnectionSettings).ConfigureAwait(false);
7574
}
7675
}
76+
requestData.MadeItToResponse = true;
7777

7878
var builder = new ResponseBuilder<TReturn>(requestData)
7979
{

‎src/Elasticsearch.Net/Exceptions/ElasticsearchClientException.cs‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
using System;
22
using System.Collections.Generic;
3+
using System.Linq;
34
using System.Text;
45

56
namespace Elasticsearch.Net
@@ -16,7 +17,7 @@ public class ElasticsearchClientException : Exception
1617

1718
public ElasticsearchClientException(string message) : base(message)
1819
{
19-
this.FailureReason = Net.PipelineFailure.Unexpected;
20+
this.FailureReason = PipelineFailure.Unexpected;
2021
}
2122

2223
public ElasticsearchClientException(PipelineFailure failure, string message, Exception innerException)
@@ -38,7 +39,13 @@ public string DebugInformation
3839
get
3940
{
4041
var sb = new StringBuilder();
41-
sb.AppendLine($"# FailureReason: {FailureReason.GetStringValue()} when trying to {Request.Method.GetStringValue()} {Request.Uri}");
42+
var failureReason = FailureReason.GetStringValue();
43+
if (this.FailureReason == PipelineFailure.Unexpected && (this.AuditTrail.HasAny()))
44+
{
45+
failureReason = this.AuditTrail.Last().Event.GetStringValue();
46+
}
47+
48+
sb.AppendLine($"# FailureReason: {failureReason} while attempting {Request.Method.GetStringValue()} {Request.Uri}");
4249
if (this.Response != null)
4350
ResponseStatics.DebugInformationBuilder(this.Response, sb);
4451
else

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

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.ComponentModel;
4-
using System.Globalization;
53
using System.Linq;
64
using System.Text;
75

@@ -32,15 +30,16 @@ public static void DebugAuditTrailExceptions(List<Audit> auditTrail, StringBuild
3230
{
3331
var auditExceptions = auditTrail.Select((audit, i) => new {audit, i}).Where(a => a.audit.Exception != null);
3432
foreach (var a in auditExceptions)
35-
sb.AppendLine($"# Audit exception in step {a.i} {a.audit.Event.GetStringValue()}:\r\n{a.audit.Exception}");
33+
sb.AppendLine($"# Audit exception in step {a.i + 1} {a.audit.Event.GetStringValue()}:\r\n{a.audit.Exception}");
3634
}
3735

3836
public static void DebugAuditTrail(List<Audit> auditTrail, StringBuilder sb)
3937
{
4038
if (auditTrail == null) return;
41-
foreach (var audit in auditTrail)
39+
foreach (var a in auditTrail.Select((a, i)=> new { a, i }))
4240
{
43-
sb.Append($" - {audit.Event.GetStringValue()}:");
41+
var audit = a.a;
42+
sb.Append($" - [{a.i + 1}] {audit.Event.GetStringValue()}:");
4443
if (audit.Node?.Uri != null) sb.Append($" Node: {audit.Node.Uri}");
4544
if (audit.Exception != null) sb.Append($" Exception: {audit.Exception.GetType().Name}");
4645
sb.AppendLine($" Took: {(audit.Ended - audit.Started)}");

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

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
11
using System;
2-
using System.Collections.Generic;
3-
using System.Linq;
4-
using System.Text;
5-
using System.Threading.Tasks;
62

73
namespace Elasticsearch.Net
84
{
@@ -13,7 +9,8 @@ public class PipelineException : Exception
139
public IApiCallDetails Response { get; internal set; }
1410

1511
public bool Recoverable =>
16-
FailureReason == PipelineFailure.BadResponse
12+
FailureReason == PipelineFailure.BadRequest
13+
|| FailureReason == PipelineFailure.BadResponse
1714
|| FailureReason == PipelineFailure.PingFailure;
1815
//|| FailureReason == FailureReason.Unexpected;
1916

@@ -39,8 +36,10 @@ private static string GetMessage(PipelineFailure failure)
3936
{
4037
switch(failure)
4138
{
39+
case PipelineFailure.BadRequest:
40+
return "An error occurred trying to write the request datato the specified node.";
4241
case PipelineFailure.BadResponse:
43-
return "An error occurred trying to establish a connection with the specified node.";
42+
return "An error occurred trying to read the response from the specified node.";
4443
case PipelineFailure.BadAuthentication:
4544
return "Could not authenticate with the specified node. Try verifying your credentials or check your Shield configuration.";
4645
case PipelineFailure.PingFailure:

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ public enum PipelineFailure
99
CouldNotStartSniffOnStartup,
1010
MaxTimeoutReached,
1111
MaxRetriesReached,
12-
Unexpected
13-
}
12+
Unexpected,
13+
BadRequest
14+
}
1415
}

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
using System.IO;
55
using System.Linq;
66
using System.Threading;
7-
using System.Threading.Tasks;
87
using Purify;
98

109
namespace Elasticsearch.Net
@@ -18,6 +17,9 @@ public class RequestData
1817
public HttpMethod Method { get; private set; }
1918
public string Path { get; }
2019
public PostData<object> PostData { get; }
20+
public bool MadeItToResponse { get; set;}
21+
public AuditEvent OnFailureAuditEvent => this.MadeItToResponse ? AuditEvent.BadResponse : AuditEvent.BadRequest;
22+
public PipelineFailure OnFailurePipelineFailure => this.MadeItToResponse ? PipelineFailure.BadResponse : PipelineFailure.BadRequest;
2123

2224
public Node Node { get; internal set; }
2325
public TimeSpan RequestTimeout { get; }
@@ -110,6 +112,7 @@ private string CreatePathWithQueryStrings(string path, IConnectionConfigurationV
110112
return path;
111113
}
112114

115+
113116
protected bool Equals(RequestData other) =>
114117
RequestTimeout.Equals(other.RequestTimeout)
115118
&& PingTimeout.Equals(other.PingTimeout)

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

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
using System.Collections.Generic;
33
using System.IO;
44
using System.Linq;
5-
using System.Net;
65
using System.Threading;
76
using System.Threading.Tasks;
87
using static Elasticsearch.Net.AuditEvent;
@@ -245,7 +244,7 @@ public void Ping(Node node)
245244
var response = this._connection.Request<VoidResponse>(pingData);
246245
ThrowBadAuthPipelineExceptionWhenNeeded(response);
247246
//ping should not silently accept bad but valid http responses
248-
if (!response.Success) throw new PipelineException(PipelineFailure.BadResponse) { Response = response };
247+
if (!response.Success) throw new PipelineException(pingData.OnFailurePipelineFailure) { Response = response };
249248
}
250249
catch (Exception e)
251250
{
@@ -269,7 +268,7 @@ public async Task PingAsync(Node node)
269268
var response = await this._connection.RequestAsync<VoidResponse>(pingData).ConfigureAwait(false);
270269
ThrowBadAuthPipelineExceptionWhenNeeded(response);
271270
//ping should not silently accept bad but valid http responses
272-
if (!response.Success) throw new PipelineException(PipelineFailure.BadResponse) { Response = response };
271+
if (!response.Success) throw new PipelineException(pingData.OnFailurePipelineFailure) { Response = response };
273272
}
274273
catch (Exception e)
275274
{
@@ -323,7 +322,7 @@ public void Sniff()
323322
var response = this._connection.Request<SniffResponse>(requestData);
324323
ThrowBadAuthPipelineExceptionWhenNeeded(response);
325324
//sniff should not silently accept bad but valid http responses
326-
if (!response.Success) throw new PipelineException(PipelineFailure.BadResponse) { Response = response };
325+
if (!response.Success) throw new PipelineException(requestData.OnFailurePipelineFailure) { Response = response };
327326
var nodes = response.Body.ToNodes(this._connectionPool.UsingSsl);
328327
this._connectionPool.Reseed(nodes);
329328
this.Refresh = true;
@@ -356,7 +355,7 @@ public async Task SniffAsync()
356355
var response = await this._connection.RequestAsync<SniffResponse>(requestData).ConfigureAwait(false);
357356
ThrowBadAuthPipelineExceptionWhenNeeded(response);
358357
//sniff should not silently accept bad but valid http responses
359-
if (!response.Success) throw new PipelineException(PipelineFailure.BadResponse) { Response = response };
358+
if (!response.Success) throw new PipelineException(requestData.OnFailurePipelineFailure) { Response = response };
360359
this._connectionPool.Reseed(response.Body.ToNodes(this._connectionPool.UsingSsl));
361360
this.Refresh = true;
362361
return;
@@ -386,13 +385,13 @@ public ElasticsearchResponse<TReturn> CallElasticsearch<TReturn>(RequestData req
386385
response = this._connection.Request<TReturn>(requestData);
387386
response.AuditTrail = this.AuditTrail;
388387
ThrowBadAuthPipelineExceptionWhenNeeded(response);
389-
if (!response.Success) audit.Event = AuditEvent.BadResponse;
388+
if (!response.Success) audit.Event = requestData.OnFailureAuditEvent;
390389
return response;
391390
}
392391
catch (Exception e)
393392
{
394393
(response as ElasticsearchResponse<Stream>)?.Body?.Dispose();
395-
audit.Event = AuditEvent.BadResponse;
394+
audit.Event = requestData.OnFailureAuditEvent;
396395
audit.Exception = e;
397396
throw;
398397
}
@@ -412,13 +411,13 @@ public async Task<ElasticsearchResponse<TReturn>> CallElasticsearchAsync<TReturn
412411
response = await this._connection.RequestAsync<TReturn>(requestData).ConfigureAwait(false);
413412
response.AuditTrail = this.AuditTrail;
414413
ThrowBadAuthPipelineExceptionWhenNeeded(response);
415-
if (!response.Success) audit.Event = AuditEvent.BadResponse;
414+
if (!response.Success) audit.Event = requestData.OnFailureAuditEvent;
416415
return response;
417416
}
418417
catch (Exception e)
419418
{
420419
(response as ElasticsearchResponse<Stream>)?.Body?.Dispose();
421-
audit.Event = AuditEvent.BadResponse;
420+
audit.Event = requestData.OnFailureAuditEvent;
422421
audit.Exception = e;
423422
throw;
424423
}
@@ -429,7 +428,7 @@ public void BadResponse<TReturn>(ref ElasticsearchResponse<TReturn> response, Re
429428
where TReturn : class
430429
{
431430
var callDetails = response ?? pipelineExceptions.LastOrDefault()?.Response;
432-
var pipelineFailure = PipelineFailure.BadResponse;
431+
var pipelineFailure = data.OnFailurePipelineFailure;
433432
if (pipelineExceptions.HasAny())
434433
pipelineFailure = pipelineExceptions.Last().FailureReason;
435434

0 commit comments

Comments
 (0)