Skip to content

try/catch when attempting to deserialize ServerError from response #3466

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Nov 7, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 23 additions & 5 deletions src/Elasticsearch.Net/Responses/ElasticsearchResponse.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ namespace Elasticsearch.Net
/// </summary>
public abstract class ElasticsearchResponseBase : IApiCallDetails, IElasticsearchResponse
{
/// <inheritdoc />
public IApiCallDetails ApiCall { get; set; }

bool IElasticsearchResponse.TryGetServerErrorReason(out string reason) => this.TryGetServerErrorReason(out reason);
Expand All @@ -20,35 +21,52 @@ protected virtual bool TryGetServerErrorReason(out string reason)
return false;
}

// TODO: Remove?
//ignored
List<Audit> IApiCallDetails.AuditTrail { get; set; }

/// <inheritdoc cref="IApiCallDetails.Success"/>
public bool Success => this.ApiCall.Success;

/// <inheritdoc cref="IApiCallDetails.ResponseMimeType"/>
public string ResponseMimeType => this.ApiCall.ResponseMimeType;

/// <inheritdoc cref="IApiCallDetails.HttpMethod"/>
public HttpMethod HttpMethod => this.ApiCall.HttpMethod;

/// <inheritdoc cref="IApiCallDetails.Uri"/>
public Uri Uri => this.ApiCall.Uri;

/// <inheritdoc cref="IApiCallDetails.HttpStatusCode"/>
public int? HttpStatusCode => this.ApiCall.HttpStatusCode;

/// <inheritdoc cref="IApiCallDetails.AuditTrail"/>
public List<Audit> AuditTrail => this.ApiCall.AuditTrail;

/// <inheritdoc cref="IApiCallDetails.DeprecationWarnings"/>
public IEnumerable<string> DeprecationWarnings => this.ApiCall.DeprecationWarnings;

/// <inheritdoc cref="IApiCallDetails.SuccessOrKnownError"/>
public bool SuccessOrKnownError => this.ApiCall.SuccessOrKnownError;

/// <inheritdoc cref="IApiCallDetails.OriginalException"/>
public Exception OriginalException => this.ApiCall.OriginalException;

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

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

/// <inheritdoc cref="IApiCallDetails.DebugInformation"/>
public string DebugInformation => this.ApiCall.DebugInformation;

public override string ToString() => this.ApiCall.ToString();


}

/// <summary>
/// A response from Elasticsearch including details about the request/response life cycle. Base class for the built in low level response
/// types: <see cref="StringResponse"/> <see cref="BytesResponse"/> and <see cref="DynamicResponse"/>
/// types, <see cref="StringResponse"/>, <see cref="BytesResponse"/>, <see cref="DynamicResponse"/> and <see cref="VoidResponse"/>
/// </summary>
public abstract class ElasticsearchResponse<T> : ElasticsearchResponseBase
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,22 @@

namespace Elasticsearch.Net
{
/// <summary>
/// Details about the API call
/// </summary>
public interface IApiCallDetails
{
/// <summary>
/// The response status code is in the 200 range or is in the allowed list of status codes set on the request.
/// </summary>
bool Success { get; }

/// <summary>The response mime type </summary>
/// <summary>The response MIME type </summary>
string ResponseMimeType { get; }

/// <summary>
/// If <see cref="Success"/> is <c>false</c>, this will hold the original exception.
/// This will be the orginating CLR exception in most cases.
/// This will be the originating CLR exception in most cases.
/// </summary>
Exception OriginalException { get; }

Expand Down Expand Up @@ -55,7 +58,7 @@ public interface IApiCallDetails
[DebuggerDisplay("{RequestBodyInBytes != null ? System.Text.Encoding.UTF8.GetString(RequestBodyInBytes) : null,nq}")]
byte[] RequestBodyInBytes { get; }

//TODO Get rid of setter
//TODO: Get rid of setter
/// <summary>
/// An audit trail of requests made to nodes within the cluster
/// </summary>
Expand Down
15 changes: 15 additions & 0 deletions src/Elasticsearch.Net/Responses/ServerException/ServerError.cs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,24 @@ public ServerError(Error error, int? statusCode)
public Error Error { get; }
public int Status { get; }

public static bool TryCreate(Stream stream, out ServerError serverError)
{
try
{
serverError = Create(stream);
return true;
}
catch
{
serverError = null;
return false;
}
}

public static ServerError Create(Stream stream) =>
LowLevelRequestResponseSerializer.Instance.Deserialize<ServerError>(stream);

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

Expand Down
10 changes: 6 additions & 4 deletions src/Elasticsearch.Net/Responses/Special/BytesResponse.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.IO;
using System;
using System.IO;
using System.Text;

namespace Elasticsearch.Net
Expand All @@ -11,10 +12,11 @@ public BytesResponse() { }
public bool TryGetServerError(out ServerError serverError)
{
serverError = null;
if (this.Body == null || this.Body.Length == 0) return false;
if (this.Body == null || this.Body.Length == 0 || ResponseMimeType != RequestData.MimeType)
return false;

using(var stream = new MemoryStream(this.Body))
serverError = ServerError.Create(stream);
return true;
return ServerError.TryCreate(stream, out serverError);
}

protected override bool TryGetServerErrorReason(out string reason)
Expand Down
10 changes: 6 additions & 4 deletions src/Elasticsearch.Net/Responses/Special/StringResponse.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.IO;
using System;
using System.IO;
using System.Text;

namespace Elasticsearch.Net
Expand All @@ -11,10 +12,11 @@ public StringResponse() { }
public bool TryGetServerError(out ServerError serverError)
{
serverError = null;
if (string.IsNullOrEmpty(this.Body)) return false;
if (string.IsNullOrEmpty(this.Body) || ResponseMimeType != RequestData.MimeType)
return false;

using(var stream = new MemoryStream(Encoding.UTF8.GetBytes(this.Body)))
serverError = ServerError.Create(stream);
return true;
return ServerError.TryCreate(stream, out serverError);
}

protected override bool TryGetServerErrorReason(out string reason)
Expand Down
23 changes: 18 additions & 5 deletions src/Tests/Tests.Core/Client/FixedResponseClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,25 @@ public static ConnectionSettings CreateConnectionSettings(
Exception exception = null)
{
var serializer = TestClient.Default.RequestResponseSerializer;
byte[] fixedResult = null;
if (response is string s) fixedResult = Encoding.UTF8.GetBytes(s);
else if (contentType == RequestData.MimeType) fixedResult = serializer.SerializeToBytes(response);
else fixedResult = Encoding.UTF8.GetBytes(response.ToString());
byte[] responseBytes;
switch (response)
{
case string s:
responseBytes = Encoding.UTF8.GetBytes(s);
break;
case byte[] b:
responseBytes = b;
break;
default:
{
responseBytes = contentType == RequestData.MimeType
? serializer.SerializeToBytes(response)
: Encoding.UTF8.GetBytes(response.ToString());
break;
}
}

var connection = new InMemoryConnection(fixedResult, statusCode, exception, contentType);
var connection = new InMemoryConnection(responseBytes, statusCode, exception, contentType);
var connectionPool = new SingleNodeConnectionPool(new Uri("http://localhost:9200"));
var defaultSettings = new ConnectionSettings(connectionPool, connection)
.DefaultIndex("default-index");
Expand Down
38 changes: 0 additions & 38 deletions src/Tests/Tests.Reproduce/BytesResponseTests.cs

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
using Nest;
using Tests.Domain;
using Tests.Framework;
using Tests.Framework.SerializationTests;

namespace Tests.ClientConcepts.ConnectionPooling.Exceptions
{
Expand Down Expand Up @@ -99,14 +100,7 @@ [U] public async Task BadAuthenticationIsUnrecoverable()
);
}

private static byte[] HtmlNginx401Response = Encoding.UTF8.GetBytes(@"<html>
<head><title>401 Authorization Required</title></head>
<body bgcolor=""white"">
<center><h1>401 Authorization Required</h1></center>
<hr><center>nginx/1.4.6 (Ubuntu)</center>
</body>
</html>
");
private static byte[] HtmlNginx401Response = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);

/**
* When a bad authentication response occurs, the client attempts to deserialize the response body returned;
Expand Down
73 changes: 73 additions & 0 deletions src/Tests/Tests/Framework/SerializationTests/BytesResponseTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
using System;
using System.Linq;
using System.Text;
using Elastic.Xunit.XunitPlumbing;
using Elasticsearch.Net;
using FluentAssertions;
using Tests.Core.Client;
using Tests.Core.ManagedElasticsearch.Clusters;

namespace Tests.Framework.SerializationTests
{
public class BytesResponseTests : IClusterFixture<ReadOnlyCluster>
{
private readonly ReadOnlyCluster _cluster;

public BytesResponseTests(ReadOnlyCluster cluster) => _cluster = cluster;

[I] public void NonNullBytesResponse()
{
var client = _cluster.Client;

var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));
bytesResponse.Body.Should().NotBeNull();
bytesResponse.Body.Should().BeEquivalentTo(bytesResponse.ResponseBodyInBytes);
}

[I] public void NonNullBytesLowLevelResponse()
{
var settings = new ConnectionConfiguration(new Uri($"http://localhost:{_cluster.Nodes.First().Port ?? 9200}"));
var lowLevelClient = new ElasticLowLevelClient(settings);

var bytesResponse = lowLevelClient.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));

bytesResponse.Body.Should().NotBeNull();
bytesResponse.Body.Should().BeEquivalentTo(bytesResponse.ResponseBodyInBytes);
}

[U]
public void TryGetServerErrorDoesNotThrowException()
{
var responseBytes = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);

var client = FixedResponseClient.Create(responseBytes, 401,
modifySettings: s => s.DisableDirectStreaming(),
contentType: "text/html",
exception: new Exception("problem with the request as a result of 401")
);

var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));

bytesResponse.Body.Should().NotBeNull();
bytesResponse.Body.Should().BeEquivalentTo(responseBytes);
bytesResponse.TryGetServerError(out var serverError).Should().BeFalse();
serverError.Should().BeNull();
}

[U] public void SkipDeserializationForStatusCodesSetsBody()
{
var responseBytes = Encoding.UTF8.GetBytes(StubResponse.NginxHtml401Response);

var client = FixedResponseClient.Create(responseBytes, 401,
modifySettings: s => s.DisableDirectStreaming().SkipDeserializationForStatusCodes(401),
contentType: "text/html",
exception: new Exception("problem with the request as a result of 401")
);

var bytesResponse = client.LowLevel.Search<BytesResponse>("project", "project", PostData.Serializable(new { }));

bytesResponse.Body.Should().NotBeNull();
bytesResponse.Body.Should().BeEquivalentTo(responseBytes);
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
using System;
using System.Text;
using Elastic.Xunit.XunitPlumbing;
using Elasticsearch.Net;
using FluentAssertions;
using Tests.Core.Client;

namespace Tests.Framework.SerializationTests
{
public class StringResponseTests
{
[U] public void TryGetServerErrorDoesNotThrowException()
{
var client = FixedResponseClient.Create(StubResponse.NginxHtml401Response, 401,
modifySettings: s => s.DisableDirectStreaming(),
contentType: "text/html",
exception: new Exception("problem with the request as a result of 401")
);

var stringResponse = client.LowLevel.Search<StringResponse>("project", "project", PostData.Serializable(new { }));

stringResponse.Body.Should().NotBeNull();
stringResponse.Body.Should().Be(StubResponse.NginxHtml401Response);
stringResponse.TryGetServerError(out var serverError).Should().BeFalse();
serverError.Should().BeNull();
}

[U] public void SkipDeserializationForStatusCodesSetsBody()
{
var client = FixedResponseClient.Create(StubResponse.NginxHtml401Response, 401,
modifySettings: s => s.DisableDirectStreaming().SkipDeserializationForStatusCodes(401),
contentType: "text/html",
exception: new Exception("problem with the request as a result of 401")
);

var stringResponse = client.LowLevel.Search<StringResponse>("project", "project", PostData.Serializable(new { }));

stringResponse.Body.Should().NotBeNull();
stringResponse.Body.Should().Be(StubResponse.NginxHtml401Response);
}
}
}
19 changes: 19 additions & 0 deletions src/Tests/Tests/Framework/SerializationTests/StubResponse.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
namespace Tests.Framework.SerializationTests
{
public class StubResponse
{
public const string NginxHtml401Response = @"<html>
<head><title>401 Authorization Required</title></head>
<body bgcolor=""white"">
<center><h1>401 Authorization Required</h1></center>
<hr><center>nginx/1.4.6 (Ubuntu)</center>
</body>
</html>
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->
<!-- a padding to disable MSIE and Chrome friendly error page -->";
}
}