Skip to content
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.

Sign up for GitHub

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

Jump to bottom

BREAKING: Backport serialization fix for replicator #968

Merged
merged 2 commits into from
Oct 8, 2024
Merged

BREAKING: Backport serialization fix for replicator #968

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6bac31
Backport serialization fix for replicator
paulirwin Oct 8, 2024
420ba30
Remove JSON_SERIALIZER_SETTINGS
paulirwin Oct 8, 2024
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
67 changes: 12 additions & 55 deletions src/Lucene.Net.Replicator/Http/HttpClientBase.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Runtime.ExceptionServices;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -53,7 +52,7 @@ public abstract class HttpClientBase : IDisposable
// TODO compression?

/// <summary>
/// The URL to execute requests against.
/// The URL to execute requests against.
/// </summary>
protected string Url { get; private set; }

Expand All @@ -64,7 +63,7 @@ public abstract class HttpClientBase : IDisposable
/// Creates a new <see cref="HttpClientBase"/> with the given host, port and path.
/// </summary>
/// <remarks>
/// The host, port and path parameters are normalized to <c>http://{host}:{port}{path}</c>,
/// The host, port and path parameters are normalized to <c>http://{host}:{port}{path}</c>,
/// if path is <c>null</c> or <c>empty</c> it defaults to <c>/</c>.
/// <para/>
/// A <see cref="HttpMessageHandler"/> is taken as an optional parameter as well, if this is not provided it defaults to <c>null</c>.
Expand Down Expand Up @@ -98,7 +97,7 @@ protected HttpClientBase(string url, HttpMessageHandler messageHandler = null)
/// Creates a new <see cref="HttpClientBase"/> with the given <paramref name="url"/> and <see cref="HttpClient"/>.
/// </summary>
/// <remarks>
/// This allows full controll over how the <see cref="HttpClient"/> is created,
/// This allows full controll over how the <see cref="HttpClient"/> is created,
/// prefer the <see cref="HttpClientBase(string, HttpMessageHandler)"/> over this unless you know you need the control of the <see cref="HttpClient"/>.
/// </remarks>
/// <param name="url"></param>
Expand All @@ -122,7 +121,7 @@ public virtual TimeSpan Timeout
}

/// <summary>
/// Throws <see cref="ObjectDisposedException"/> if this client is already disposed.
/// Throws <see cref="ObjectDisposedException"/> if this client is already disposed.
/// </summary>
/// <exception cref="ObjectDisposedException">client is already disposed.</exception>
protected void EnsureOpen()
Expand Down Expand Up @@ -166,51 +165,9 @@ protected virtual void VerifyStatus(HttpResponseMessage response)
/// <summary>
/// Throws an exception for any errors.
/// </summary>
/// <exception cref="IOException">IO Error happened at the server, check inner exception for details.</exception>
/// <exception cref="HttpRequestException">Unknown error received from the server.</exception>
protected virtual void ThrowKnownError(HttpResponseMessage response)
{
Stream input;
try
{
//.NET Note: Bridging from Async to Sync, this is not ideal and we could consider changing the interface to be Async or provide Async overloads
// and have these Sync methods with their caveats.
input = response.Content.ReadAsStreamAsync().ConfigureAwait(false).GetAwaiter().GetResult();
}
catch (Exception t) when (t.IsThrowable())
{
// the response stream is not an exception - could be an error in servlet.init().
// LUCENENET: Check status code to see if we had an HTTP error
try
{
response.EnsureSuccessStatusCode();
}
catch (HttpRequestException e)
{
throw RuntimeException.Create(e);
}

throw RuntimeException.Create("Unknown error: ", t);
}

Exception exception;
try
{
TextReader reader = new StreamReader(input);
JsonSerializer serializer = JsonSerializer.Create(ReplicationService.JSON_SERIALIZER_SETTINGS);
exception = (Exception)serializer.Deserialize(new JsonTextReader(reader));
}
catch (Exception th) when (th.IsThrowable())
{
//not likely
throw RuntimeException.Create(string.Format("Failed to read exception object: {0} {1}", response.StatusCode, response.ReasonPhrase), th);
}
finally
{
input.Dispose();
}

Util.IOUtils.ReThrow(exception);
throw RuntimeException.Create(response.ReasonPhrase ?? $"Unknown error: {response.StatusCode}");
}

/// <summary>
Expand All @@ -224,7 +181,7 @@ protected virtual HttpResponseMessage ExecutePost(string request, object entity,
//.NET Note: No headers? No ContentType?... Bad use of Http?
HttpRequestMessage req = new HttpRequestMessage(HttpMethod.Post, QueryString(request, parameters));

req.Content = new StringContent(JToken.FromObject(entity, JsonSerializer.Create(ReplicationService.JSON_SERIALIZER_SETTINGS))
req.Content = new StringContent(JToken.FromObject(entity, JsonSerializer.Create())
.ToString(Formatting.None), Encoding.UTF8, "application/json");

return Execute(req);
Expand Down Expand Up @@ -272,7 +229,7 @@ public virtual Stream ResponseInputStream(HttpResponseMessage response)

// TODO: can we simplify this Consuming !?!?!?
/// <summary>
/// Internal utility: input stream of the provided response, which optionally
/// Internal utility: input stream of the provided response, which optionally
/// consumes the response's resources when the input stream is exhausted.
/// </summary>
/// <exception cref="IOException"></exception>
Expand All @@ -293,7 +250,7 @@ public virtual Stream GetResponseStream(HttpResponseMessage response) // LUCENEN

// TODO: can we simplify this Consuming !?!?!?
/// <summary>
/// Internal utility: input stream of the provided response, which optionally
/// Internal utility: input stream of the provided response, which optionally
/// consumes the response's resources when the input stream is exhausted.
/// </summary>
/// <exception cref="IOException"></exception>
Expand Down Expand Up @@ -321,7 +278,7 @@ protected virtual T DoAction<T>(HttpResponseMessage response, Func<T> call)
}

/// <summary>
/// Do a specific action and validate after the action that the status is still OK,
/// Do a specific action and validate after the action that the status is still OK,
/// and if not, attempt to extract the actual server side exception. Optionally
/// release the response at exit, depending on <paramref name="consume"/> parameter.
/// </summary>
Expand Down Expand Up @@ -359,11 +316,11 @@ protected virtual T DoAction<T>(HttpResponseMessage response, bool consume, Func
}
if (Debugging.AssertsEnabled) Debugging.Assert(th != null); // extra safety - if we get here, it means the Func<T> failed
Util.IOUtils.ReThrow(th);
return default; // silly, if we're here, IOUtils.reThrow always throws an exception
return default; // silly, if we're here, IOUtils.reThrow always throws an exception
}

/// <summary>
/// Disposes this <see cref="HttpClientBase"/>.
/// Disposes this <see cref="HttpClientBase"/>.
/// When called with <code>true</code>, this disposes the underlying <see cref="HttpClient"/>.
/// </summary>
protected virtual void Dispose(bool disposing)
Expand All @@ -376,7 +333,7 @@ protected virtual void Dispose(bool disposing)
}

/// <summary>
/// Disposes this <see cref="HttpClientBase"/>.
/// Disposes this <see cref="HttpClientBase"/>.
/// This disposes the underlying <see cref="HttpClient"/>.
/// </summary>
public void Dispose()
Expand Down
23 changes: 2 additions & 21 deletions src/Lucene.Net.Replicator/Http/ReplicationService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,15 +81,6 @@ public enum ReplicationAction
/// </summary>
public const string REPLICATE_FILENAME_PARAM = "filename";

/// <summary>
/// Json Serializer Settings to use when serializing and deserializing errors.
/// </summary>
// LUCENENET specific
public static readonly JsonSerializerSettings JSON_SERIALIZER_SETTINGS = new JsonSerializerSettings()
paulirwin marked this conversation as resolved.
Show resolved Hide resolved
{
TypeNameHandling = TypeNameHandling.All
};

private const int SHARD_IDX = 0, ACTION_IDX = 1;

private readonly string context;
Expand Down Expand Up @@ -188,24 +179,14 @@ public virtual void Perform(IReplicationRequest request, IReplicationResponse re
break;
}
}
catch (Exception e)
catch (Exception)
{
response.StatusCode = (int)HttpStatusCode.InternalServerError; // propagate the failure
try
{
TextWriter writer = new StreamWriter(response.Body);
JsonSerializer serializer = JsonSerializer.Create(JSON_SERIALIZER_SETTINGS);
NightOwl888 marked this conversation as resolved.
Show resolved Hide resolved
serializer.Serialize(writer, e, e.GetType());
}
catch (Exception e2) when (e2.IsException())
{
throw new IOException("Could not serialize", e2);
}
}
finally
{
response.Flush();
}
}
}
}
}
Loading