Skip to content

Commit

Permalink
BREAKING: Backport serialization fix for replicator (#968)
Browse files Browse the repository at this point in the history 
* Backport serialization fix for replicator

* Remove JSON_SERIALIZER_SETTINGS on ReplicationService (breaking change)
  • Loading branch information
@paulirwin
paulirwin authored Oct 8, 2024
1 parent e13721c commit 1f61dd0
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 76 deletions.
67 changes: 12 additions & 55 deletions src/Lucene.Net.Replicator/Http/HttpClientBase.cs
View file
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
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()
{
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);
serializer.Serialize(writer, e, e.GetType());
}
catch (Exception e2) when (e2.IsException())
{
throw new IOException("Could not serialize", e2);
}
}
finally
{
response.Flush();
}
}
}
}
}

0 comments on commit 1f61dd0

Please sign in to comment.