Skip to content

Commit 42414ec

Browse files
russcamrusscam
committed
try/catch when attempting to deserialize ServerError from response
This commit adds a try/catch block around attempting to deserialize a ServerError from the body, since IElasticsearchResponse types withn Elasticsearch.Net set/attempt to set the response body, irrespective of the response status code. Move BytesResponseTests and StringResponseTests from Tests.Reproduce into main Tests project. Allow byte array to be passed to FixedResponseClient. Fixes #3378 (cherry picked from commit 48304e3) Add ServerError.TryCreate() method (cherry picked from commit 552e004) Don't attempt to deserialize when not application/json MIME type (cherry picked from commit 2ef5f46)
1 parent b3805ad commit 42414ec

File tree

11 files changed

+209
-67
lines changed

11 files changed

+209
-67
lines changed

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

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,27 +8,46 @@ namespace Elasticsearch.Net
88
/// </summary>
99
public abstract class ElasticsearchResponseBase : IApiCallDetails, IElasticsearchResponse
1010
{
11+
/// <inheritdoc />
1112
public IApiCallDetails ApiCall { get; set; }
13+
14+
/// <inheritdoc cref="IApiCallDetails.AuditTrail"/>
1215
public List<Audit> AuditTrail => ApiCall.AuditTrail;
1316

17+
/// <inheritdoc cref="IApiCallDetails.DebugInformation"/>
1418
public string DebugInformation => ApiCall.DebugInformation;
19+
20+
/// <inheritdoc cref="IApiCallDetails.DeprecationWarnings"/>
1521
public IEnumerable<string> DeprecationWarnings => ApiCall.DeprecationWarnings;
22+
23+
/// <inheritdoc cref="IApiCallDetails.HttpMethod"/>
1624
public HttpMethod HttpMethod => ApiCall.HttpMethod;
25+
26+
/// <inheritdoc cref="IApiCallDetails.HttpStatusCode"/>
1727
public int? HttpStatusCode => ApiCall.HttpStatusCode;
28+
29+
/// <inheritdoc cref="IApiCallDetails.OriginalException"/>
1830
public Exception OriginalException => ApiCall.OriginalException;
1931

20-
/// <summary>The raw byte request message body, only set when DisableDirectStreaming() is set on Connection configuration</summary>
32+
/// <inheritdoc cref="IApiCallDetails.RequestBodyInBytes"/>
2133
public byte[] RequestBodyInBytes => ApiCall.RequestBodyInBytes;
2234

23-
/// <summary>The raw byte response message body, only set when DisableDirectStreaming() is set on Connection configuration</summary>
35+
/// <inheritdoc cref="IApiCallDetails.ResponseBodyInBytes"/>
2436
public byte[] ResponseBodyInBytes => ApiCall.ResponseBodyInBytes;
2537

38+
/// <inheritdoc cref="IApiCallDetails.ResponseMimeType"/>
2639
public string ResponseMimeType => ApiCall.ResponseMimeType;
2740

41+
/// <inheritdoc cref="IApiCallDetails.Success"/>
2842
public bool Success => ApiCall.Success;
43+
44+
/// <inheritdoc cref="IApiCallDetails.SuccessOrKnownError"/>
2945
public bool SuccessOrKnownError => ApiCall.SuccessOrKnownError;
46+
47+
/// <inheritdoc cref="IApiCallDetails.Uri"/>
3048
public Uri Uri => ApiCall.Uri;
3149

50+
// TODO: Remove?
3251
//ignored
3352
List<Audit> IApiCallDetails.AuditTrail { get; set; }
3453

@@ -45,7 +64,7 @@ protected virtual bool TryGetServerErrorReason(out string reason)
4564

4665
/// <summary>
4766
/// A response from Elasticsearch including details about the request/response life cycle. Base class for the built in low level response
48-
/// types: <see cref="StringResponse" /> <see cref="BytesResponse" /> and <see cref="DynamicResponse" />
67+
/// types, <see cref="StringResponse"/>, <see cref="BytesResponse"/>, <see cref="DynamicResponse"/> and <see cref="VoidResponse"/>
4968
/// </summary>
5069
public abstract class ElasticsearchResponse<T> : ElasticsearchResponseBase
5170
{

‎src/Elasticsearch.Net/Responses/HttpDetails/IApiCallDetails.cs‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,17 @@
44

55
namespace Elasticsearch.Net
66
{
7+
/// <summary>
8+
/// Details about the API call
9+
/// </summary>
710
public interface IApiCallDetails
811
{
9-
//TODO Get rid of setter
10-
/// <summary>
11-
/// An audit trail of requests made to nodes within the cluster
12-
/// </summary>
13-
List<Audit> AuditTrail { get; set; }
12+
13+
//TODO: Get rid of setter
14+
/// <summary>
15+
/// An audit trail of requests made to nodes within the cluster
16+
/// </summary>
17+
List<Audit> AuditTrail { get; set; }
1418

1519
/// <summary>
1620
/// A lazy human readable string representation of what happened during this request for both successful and
@@ -35,8 +39,8 @@ public interface IApiCallDetails
3539
int? HttpStatusCode { get; }
3640

3741
/// <summary>
38-
/// If <see cref="Success" /> is <c>false</c>, this will hold the original exception.
39-
/// This will be the orginating CLR exception in most cases.
42+
/// If <see cref="Success"/> is <c>false</c>, this will hold the original exception.
43+
/// This will be the originating CLR exception in most cases.
4044
/// </summary>
4145
Exception OriginalException { get; }
4246

@@ -54,7 +58,7 @@ public interface IApiCallDetails
5458
[DebuggerDisplay("{ResponseBodyInBytes != null ? System.Text.Encoding.UTF8.GetString(ResponseBodyInBytes) : null,nq}")]
5559
byte[] ResponseBodyInBytes { get; }
5660

57-
/// <summary>The response mime type </summary>
61+
/// <summary>The response MIME type </summary>
5862
string ResponseMimeType { get; }
5963

6064
/// <summary>

‎src/Elasticsearch.Net/Responses/ServerException/ServerError.cs‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,24 @@ public ServerError(Error error, int? statusCode)
1818
public Error Error { get; }
1919
public int Status { get; }
2020

21+
public static bool TryCreate(Stream stream, out ServerError serverError)
22+
{
23+
try
24+
{
25+
serverError = Create(stream);
26+
return true;
27+
}
28+
catch
29+
{
30+
serverError = null;
31+
return false;
32+
}
33+
}
34+
2135
public static ServerError Create(Stream stream) =>
2236
LowLevelRequestResponseSerializer.Instance.Deserialize<ServerError>(stream);
2337

38+
// TODO: make token default parameter in 7.x
2439
public static Task<ServerError> CreateAsync(Stream stream, CancellationToken token) =>
2540
LowLevelRequestResponseSerializer.Instance.DeserializeAsync<ServerError>(stream, token);
2641

‎src/Elasticsearch.Net/Responses/Special/BytesResponse.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,11 @@ public BytesResponse() { }
1111
public bool TryGetServerError(out ServerError serverError)
1212
{
1313
serverError = null;
14-
if (Body == null || Body.Length == 0) return false;
14+
if (Body == null || Body.Length == 0 || ResponseMimeType != RequestData.MimeType)
15+
return false;
1516

1617
using (var stream = new MemoryStream(Body))
17-
serverError = ServerError.Create(stream);
18-
return true;
18+
return ServerError.TryCreate(stream, out serverError);
1919
}
2020

2121
protected override bool TryGetServerErrorReason(out string reason)

‎src/Elasticsearch.Net/Responses/Special/StringResponse.cs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,11 +12,11 @@ public StringResponse() { }
1212
public bool TryGetServerError(out ServerError serverError)
1313
{
1414
serverError = null;
15-
if (string.IsNullOrEmpty(Body)) return false;
15+
if (string.IsNullOrEmpty(Body) || ResponseMimeType != RequestData.MimeType)
16+
return false;
1617

1718
using (var stream = new MemoryStream(Encoding.UTF8.GetBytes(Body)))
18-
serverError = ServerError.Create(stream);
19-
return true;
19+
return ServerError.TryCreate(stream, out serverError);
2020
}
2121

2222
protected override bool TryGetServerErrorReason(out string reason)

‎src/Tests/Tests.Core/Client/FixedResponseClient.cs‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,12 +28,25 @@ public static ConnectionSettings CreateConnectionSettings(
2828
)
2929
{
3030
var serializer = TestClient.Default.RequestResponseSerializer;
31-
byte[] fixedResult = null;
32-
if (response is string s) fixedResult = Encoding.UTF8.GetBytes(s);
33-
else if (contentType == RequestData.MimeType) fixedResult = serializer.SerializeToBytes(response);
34-
else fixedResult = Encoding.UTF8.GetBytes(response.ToString());
31+
byte[] responseBytes;
32+
switch (response)
33+
{
34+
case string s:
35+
responseBytes = Encoding.UTF8.GetBytes(s);
36+
break;
37+
case byte[] b:
38+
responseBytes = b;
39+
break;
40+
default:
41+
{
42+
responseBytes = contentType == RequestData.MimeType
43+
? serializer.SerializeToBytes(response)
44+
: Encoding.UTF8.GetBytes(response.ToString());
45+
break;
46+
}
47+
}
3548

36-
var connection = new InMemoryConnection(fixedResult, statusCode, exception, contentType);
49+
var connection = new InMemoryConnection(responseBytes, statusCode, exception, contentType);
3750
var connectionPool = new SingleNodeConnectionPool(new Uri("http://localhost:9200"));
3851
var defaultSettings = new ConnectionSettings(connectionPool, connection)
3952
.DefaultIndex("default-index");

‎src/Tests/Tests.Reproduce/BytesResponseTests.cs‎

Lines changed: 0 additions & 37 deletions
This file was deleted.

‎src/Tests/Tests/ClientConcepts/ConnectionPooling/Exceptions/UnrecoverableExceptions.doc.cs‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
using Nest;
1010
using Tests.Domain;
1111
using Tests.Framework;
12+
using Tests.Framework.SerializationTests;
1213

1314
namespace Tests.ClientConcepts.ConnectionPooling.Exceptions
1415
{
@@ -99,14 +100,7 @@ [U] public async Task BadAuthenticationIsUnrecoverable()
99100
);
100101
}
101102

102-
private static byte[] HtmlNginx401Response = Encoding.UTF8.GetBytes(@"<html>
103-
<head><title>401 Authorization Required</title></head>
104-
<body bgcolor=""white"">
105-
<center><h1>401 Authorization Required</h1></center>
106-
<hr><center>nginx/1.4.6 (Ubuntu)</center>
107-
</body>
108-
</html>
109-
");
103+
private static byte[] HtmlNginx401Response = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);
110104

111105
/**
112106
* When a bad authentication response occurs, the client attempts to deserialize the response body returned;

‎src/Tests/Tests/Framework/SerializationTests/BytesResponseTests.cs‎

Lines changed: 73 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,73 @@
1+
using System;
2+
using System.Linq;
3+
using System.Text;
4+
using Elastic.Xunit.XunitPlumbing;
5+
using Elasticsearch.Net;
6+
using FluentAssertions;
7+
using Tests.Core.Client;
8+
using Tests.Core.ManagedElasticsearch.Clusters;
9+
10+
namespace Tests.Framework.SerializationTests
11+
{
12+
public class BytesResponseTests : IClusterFixture<ReadOnlyCluster>
13+
{
14+
private readonly ReadOnlyCluster _cluster;
15+
16+
public BytesResponseTests(ReadOnlyCluster cluster) => _cluster = cluster;
17+
18+
[I] public void NonNullBytesResponse()
19+
{
20+
var client = _cluster.Client;
21+
22+
var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
23+
bytesResponse.Body.Should().NotBeNull();
24+
bytesResponse.Body.Should().BeEquivalentTo(bytesResponse.ResponseBodyInBytes);
25+
}
26+
27+
[I] public void NonNullBytesLowLevelResponse()
28+
{
29+
var settings = new ConnectionConfiguration(new Uri($"http://localhost:{_cluster.Nodes.First().Port ?? 9200}"));
30+
var lowLevelClient = new ElasticLowLevelClient(settings);
31+
32+
var bytesResponse = lowLevelClient.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
33+
34+
bytesResponse.Body.Should().NotBeNull();
35+
bytesResponse.Body.Should().BeEquivalentTo(bytesResponse.ResponseBodyInBytes);
36+
}
37+
38+
[U]
39+
public void TryGetServerErrorDoesNotThrowException()
40+
{
41+
var responseBytes = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);
42+
43+
var client = FixedResponseClient.Create(responseBytes, 401,
44+
modifySettings: s => s.DisableDirectStreaming(),
45+
contentType: "text/html",
46+
exception: new Exception("problem with the request as a result of 401")
47+
);
48+
49+
var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
50+
51+
bytesResponse.Body.Should().NotBeNull();
52+
bytesResponse.Body.Should().BeEquivalentTo(responseBytes);
53+
bytesResponse.TryGetServerError(out var serverError).Should().BeFalse();
54+
serverError.Should().BeNull();
55+
}
56+
57+
[U] public void SkipDeserializationForStatusCodesSetsBody()
58+
{
59+
var responseBytes = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);
60+
61+
var client = FixedResponseClient.Create(responseBytes, 401,
62+
modifySettings: s => s.DisableDirectStreaming().SkipDeserializationForStatusCodes(401),
63+
contentType: "text/html",
64+
exception: new Exception("problem with the request as a result of 401")
65+
);
66+
67+
var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
68+
69+
bytesResponse.Body.Should().NotBeNull();
70+
bytesResponse.Body.Should().BeEquivalentTo(responseBytes);
71+
}
72+
}
73+
}

‎src/Tests/Tests/Framework/SerializationTests/StringResponseTests.cs‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
using System;
2+
using System.Text;
3+
using Elastic.Xunit.XunitPlumbing;
4+
using Elasticsearch.Net;
5+
using FluentAssertions;
6+
using Tests.Core.Client;
7+
8+
namespace Tests.Framework.SerializationTests
9+
{
10+
public class StringResponseTests
11+
{
12+
[U] public void TryGetServerErrorDoesNotThrowException()
13+
{
14+
var client = FixedResponseClient.Create(StubResponse.NginxHtml401Response, 401,
15+
modifySettings: s => s.DisableDirectStreaming(),
16+
contentType: "text/html",
17+
exception: new Exception("problem with the request as a result of 401")
18+
);
19+
20+
var stringResponse = client.LowLevel.Search<StringResponse>("project", "project", PostData.Serializable(new { }));
21+
22+
stringResponse.Body.Should().NotBeNull();
23+
stringResponse.Body.Should().Be(StubResponse.NginxHtml401Response);
24+
stringResponse.TryGetServerError(out var serverError).Should().BeFalse();
25+
serverError.Should().BeNull();
26+
}
27+
28+
[U] public void SkipDeserializationForStatusCodesSetsBody()
29+
{
30+
var client = FixedResponseClient.Create(StubResponse.NginxHtml401Response, 401,
31+
modifySettings: s => s.DisableDirectStreaming().SkipDeserializationForStatusCodes(401),
32+
contentType: "text/html",
33+
exception: new Exception("problem with the request as a result of 401")
34+
);
35+
36+
var stringResponse = client.LowLevel.Search<StringResponse>("project", "project", PostData.Serializable(new { }));
37+
38+
stringResponse.Body.Should().NotBeNull();
39+
stringResponse.Body.Should().Be(StubResponse.NginxHtml401Response);
40+
}
41+
}
42+
}

0 commit comments

Comments
 (0)