Skip to content

Commit

Permalink
Consolidate/refactor local exceptions in C# (#2338)
Browse files Browse the repository at this point in the history
  • Loading branch information
@bernardnormier
bernardnormier authored Jun 25, 2024
1 parent 968ed6b commit 0831ab1
Show file tree
Hide file tree
Showing 71 changed files with 1,210 additions and 2,968 deletions.
31 changes: 31 additions & 0 deletions CHANGELOG-3.8.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,37 @@ These are the changes since the Ice 3.7.10 release in [CHANGELOG-3.7.md](./CHANG
(the unit is seconds). You can also override this value for a specific object adapter with the configuration
property `AdapterName.Connection.CloseTimeout`.

- Consolidate and refactor the exceptions derived from LocalException.
| Local exception in Ice 3.7 | Replacement |
|-------------------------------------|----------------------------|
| BadMagicException | ProtocolException (base) |
| CompressionException | ProtocolException (base) |
| ConnectionManuallyClosedException | ConnectionAbortedException, ConnectionClosedException |
| ConnectionNotValidatedException | ProtocolException (base) |
| EncapsulationException | MarshalException (base) |
| EndpointParseException | ParseException |
| EndpointSelectionTypeParseException | ParseException |
| IllegalIdentityException | ArgumentException (C#) |
| IllegalMessageSizeException | MarshalException |
| IllegalServantException | ArgumentNullException (C#) |
| IdentityParseException | ParseException |
| MemoryLimitException | MarshalException (base) |
| NoValueFactoryException | MarshalException (base) |
| ProxyParseException | ParseException |
| ProxyUnmarshalException | MarshalException (base) |
| UnexpectedObjectException | MarshalException (base) |
| UnknownMessageException | ProtocolException (base) |
| UnknownReplyStatusException | MarshalException |
| UnmarshalOutOfBoundsException | MarshalException (base) |
| UnsupportedEncodingException | MarshalException |
| UnsupportedProtocolException | MarshalException, FeatureNotSupportedException |
| VersionParseException | ParseException |

base = was existing base class

New local exceptions:\
ConnectionAbortedException, ConnectionClosedException, ConnectionIdleException, ParseException

## Slice Language Changes

- Removed support for the `["preserve-slice"]` metadata. Slice classes encoded in the Sliced-format are now always
Expand Down
12 changes: 8 additions & 4 deletions cpp/test/Ice/proxy/AllTests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1008,8 +1008,10 @@ allTests(TestHelper* helper)
}
catch (const Ice::UnknownLocalException& ex)
{
// The server thrown an UnsupportedEncodingException
test(ex.unknown.find("UnsupportedEncodingException") != string::npos);
// TODO: remove UnsupportedEncodingException
test(
ex.unknown.find("MarshalException") != string::npos ||
ex.unknown.find("UnsupportedEncodingException") != string::npos);
}

try
Expand All @@ -1029,8 +1031,10 @@ allTests(TestHelper* helper)
}
catch (const Ice::UnknownLocalException& ex)
{
// The server thrown an UnsupportedEncodingException
test(ex.unknown.find("UnsupportedEncodingException") != string::npos);
// TODO: remove UnsupportedEncodingException
test(
ex.unknown.find("MarshalException") != string::npos ||
ex.unknown.find("UnsupportedEncodingException") != string::npos);
}

cout << "ok" << endl;
Expand Down
5 changes: 2 additions & 3 deletions csharp/src/Ice/Communicator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,8 @@ public bool isShutdown()
/// Converts a stringified proxy into a proxy.
/// For example, "MyCategory/MyObject:tcp -h some_host -p 10000" creates a proxy that refers to the Ice object
/// having an identity with a name "MyObject" and a category "MyCategory", with the server running on host
/// "some_host", port 10000. If the stringified proxy does not parse correctly, this method throws one of
/// ProxyParseException, EndpointParseException, or IdentityParseException. Refer to the Ice manual for a detailed
/// description of the syntax supported by stringified proxies.
/// "some_host", port 10000. If the stringified proxy does not parse correctly, this method throws ParseException.
/// Refer to the Ice manual for a detailed description of the syntax supported by stringified proxies.
/// </summary>
/// <param name="str">The stringified proxy to convert into a proxy.</param>
/// <returns>The proxy, or null if str is an empty string.</returns>
Expand Down
2 changes: 1 addition & 1 deletion csharp/src/Ice/Connection.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ System.Threading.Tasks.Task heartbeatAsync(
/// Throw an exception indicating the reason for connection closure.
/// For example,
/// CloseConnectionException is raised if the connection was closed gracefully, whereas
/// ConnectionManuallyClosedException is raised if the connection was manually closed by
/// ConnectionAbortedException/ConnectionClosedException is raised if the connection was manually closed by
/// the application. This operation does nothing if the connection is not yet closed.
/// </summary>
void throwException();
Expand Down
106 changes: 66 additions & 40 deletions csharp/src/Ice/ConnectionI.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ internal void destroy(int reason)
{
case ObjectAdapterDeactivated:
{
setState(StateClosing, new ObjectAdapterDeactivatedException());
setState(StateClosing, new ObjectAdapterDeactivatedException(_adapter?.getName() ?? ""));
break;
}

Expand All @@ -161,11 +161,18 @@ public void close(ConnectionClose mode)
{
if (mode == ConnectionClose.Forcefully)
{
setState(StateClosed, new ConnectionManuallyClosedException(false));
setState(StateClosed,
new ConnectionAbortedException(
"Connection close forcefully by the application.",
closedByApplication: true));
}
else if (mode == ConnectionClose.Gracefully)
{
setState(StateClosing, new ConnectionManuallyClosedException(true));
setState(
StateClosing,
new ConnectionClosedException(
"Connection close gracefully by the application.",
closedByApplication: true));
}
else
{
Expand All @@ -179,7 +186,11 @@ public void close(ConnectionClose mode)
Monitor.Wait(this);
}

setState(StateClosing, new ConnectionManuallyClosedException(true));
setState(
StateClosing,
new ConnectionClosedException(
"Connection close gracefully by the application.",
closedByApplication: true));
}
}
}
Expand Down Expand Up @@ -884,7 +895,7 @@ public override void message(ref ThreadPoolCurrent current)
//
// This situation is possible for small UDP packets.
//
throw new IllegalMessageSizeException();
throw new MarshalException("Received Ice message with too few bytes in header.");
}

// Decode the header.
Expand All @@ -897,22 +908,29 @@ public override void message(ref ThreadPoolCurrent current)
if (m[0] != Protocol.magic[0] || m[1] != Protocol.magic[1] ||
m[2] != Protocol.magic[2] || m[3] != Protocol.magic[3])
{
BadMagicException ex = new BadMagicException();
ex.badMagic = m;
throw ex;
throw new ProtocolException(
$"Bad magic in message header: {m[0]:X2} {m[1]:X2} {m[2]:X2} {m[3]:X2}");
}

ProtocolVersion pv = new ProtocolVersion(_readStream);
Protocol.checkSupportedProtocol(pv);
EncodingVersion ev = new EncodingVersion(_readStream);
Protocol.checkSupportedProtocolEncoding(ev);
var pv = new ProtocolVersion(_readStream);
if (pv != Util.currentProtocol)
{
throw new MarshalException(
$"Invalid protocol version in message header: {pv.major}.{pv.minor}");
}
var ev = new EncodingVersion(_readStream);
if (ev != Util.currentProtocolEncoding)
{
throw new MarshalException(
$"Invalid protocol encoding version in message header: {ev.major}.{ev.minor}");
}

_readStream.readByte(); // messageType
_readStream.readByte(); // compress
int size = _readStream.readInt();
if (size < Protocol.headerSize)
{
throw new IllegalMessageSizeException();
throw new MarshalException($"Received Ice message with unexpected size {size}.");
}

// Resize the read buffer to the message size.
Expand Down Expand Up @@ -1249,7 +1267,7 @@ private void finish()
// Trace the cause of unexpected connection closures
//
if (!(_exception is CloseConnectionException ||
_exception is ConnectionManuallyClosedException ||
_exception is ConnectionAbortedException ||
_exception is ConnectionClosedException ||
_exception is ConnectionIdleException ||
_exception is CommunicatorDestroyedException ||
Expand Down Expand Up @@ -1507,16 +1525,16 @@ internal void idleCheck(TimeSpan idleTimeout)
{
if (isActiveOrHolding())
{
int idleTimeoutInSeconds = (int)idleTimeout.TotalSeconds;

if (_instance.traceLevels().network >= 1)
{
int idleTimeoutInSeconds = (int)idleTimeout.TotalSeconds;

_instance.initializationData().logger.trace(
_instance.traceLevels().networkCat,
$"connection aborted by the idle check because it did not receive any bytes for {idleTimeoutInSeconds}s\n{_transceiver.toDetailedString()}");
}

setState(StateClosed, new ConnectionIdleException());
setState(StateClosed, new ConnectionIdleException($"Connection aborted by the idle check because it did not receive any bytes for {idleTimeoutInSeconds}s."));
}
// else nothing to do
}
Expand Down Expand Up @@ -1590,7 +1608,7 @@ private void setState(int state, LocalException ex)
// Don't warn about certain expected exceptions.
//
if (!(_exception is CloseConnectionException ||
_exception is ConnectionManuallyClosedException ||
_exception is ConnectionAbortedException ||
_exception is ConnectionClosedException ||
_exception is ConnectionIdleException ||
_exception is CommunicatorDestroyedException ||
Expand Down Expand Up @@ -1753,7 +1771,7 @@ private void setState(int state)
if (_observer is not null && state == StateClosed && _exception is not null)
{
if (!(_exception is CloseConnectionException ||
_exception is ConnectionManuallyClosedException ||
_exception is ConnectionAbortedException ||
_exception is ConnectionClosedException ||
_exception is ConnectionIdleException ||
_exception is CommunicatorDestroyedException ||
Expand Down Expand Up @@ -1921,27 +1939,34 @@ private bool validate(int operation)
if (m[0] != Protocol.magic[0] || m[1] != Protocol.magic[1] ||
m[2] != Protocol.magic[2] || m[3] != Protocol.magic[3])
{
BadMagicException ex = new BadMagicException();
ex.badMagic = m;
throw ex;
throw new ProtocolException(
$"Bad magic in message header: {m[0]:X2} {m[1]:X2} {m[2]:X2} {m[3]:X2}");
}

ProtocolVersion pv = new ProtocolVersion(_readStream);
Protocol.checkSupportedProtocol(pv);

EncodingVersion ev = new EncodingVersion(_readStream);
Protocol.checkSupportedProtocolEncoding(ev);
var pv = new ProtocolVersion(_readStream);
if (pv != Util.currentProtocol)
{
throw new MarshalException(
$"Invalid protocol version in message header: {pv.major}.{pv.minor}");
}
var ev = new EncodingVersion(_readStream);
if (ev != Util.currentProtocolEncoding)
{
throw new MarshalException(
$"Invalid protocol encoding version in message header: {ev.major}.{ev.minor}");
}

byte messageType = _readStream.readByte();
if (messageType != Protocol.validateConnectionMsg)
{
throw new ConnectionNotValidatedException();
throw new ProtocolException(
$"Received message of type {messageType} on connection that is not yet validated.");
}
_readStream.readByte(); // Ignore compression status for validate connection.
int size = _readStream.readInt();
if (size != Protocol.headerSize)
{
throw new IllegalMessageSizeException();
throw new MarshalException($"Received ValidateConnection message with unexpected size {size}.");
}
TraceUtil.traceRecv(_readStream, _logger, _traceLevels);
}
Expand Down Expand Up @@ -2288,9 +2313,7 @@ private int parseMessage(ref MessageInfo info)
else
{
string lib = AssemblyUtil.isWindows ? "bzip2.dll" : "libbz2.so.1";
FeatureNotSupportedException ex = new FeatureNotSupportedException();
ex.unsupportedFeature = "Cannot uncompress compressed message: " + lib + " not found";
throw ex;
throw new FeatureNotSupportedException($"Cannot uncompress compressed message: {lib} not found");
}
}
info.stream.pos(Protocol.headerSize);
Expand Down Expand Up @@ -2364,12 +2387,12 @@ private int parseMessage(ref MessageInfo info)
else
{
TraceUtil.traceRecv(info.stream, _logger, _traceLevels);
info.requestCount = info.stream.readInt();
if (info.requestCount < 0)
int requestCount = info.stream.readInt();
if (requestCount < 0)
{
info.requestCount = 0;
throw new UnmarshalOutOfBoundsException();
throw new MarshalException($"Received batch request with {requestCount} batches.");
}
info.requestCount = requestCount;
info.adapter = _adapter;
info.upcallCount += info.requestCount;

Expand Down Expand Up @@ -2427,7 +2450,8 @@ private int parseMessage(ref MessageInfo info)
{
TraceUtil.trace("received unknown message\n(invalid, closing connection)",
info.stream, _logger, _traceLevels);
throw new UnknownMessageException();

throw new ProtocolException($"Received Ice protocol message with unknown type: {messageType}");
}
}
}
Expand Down Expand Up @@ -2618,9 +2642,11 @@ private void inactivityCheck(System.Threading.Timer inactivityTimer)

if (_state == StateActive)
{
// TODO: fix LocalException to accept a message
// "connection closed because it remained inactive for longer than the inactivity timeout"
setState(StateClosing, new ConnectionClosedException());
setState(
StateClosing,
new ConnectionClosedException(
"Connection closed because it remained inactive for longer than the inactivity timeout.",
closedByApplication: false));
}
}
// Else this timer was already canceled and disposed. Nothing to do.
Expand Down
29 changes: 11 additions & 18 deletions csharp/src/Ice/CurrentExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -157,39 +157,32 @@ OutgoingResponse createOutgoingResponseCore(System.Exception exc)
_ => throw new Ice.MarshalException("Unexpected exception type")
};

if (rfe.id.name.Length == 0)
Identity id = rfe.id;
string facet = rfe.facet;
if (id.name.Length == 0)
{
rfe.id = current.id;
id = current.id;
facet = current.facet;
}
string operation = rfe.operation.Length == 0 ? current.operation : rfe.operation;

if (rfe.facet.Length == 0 && current.facet.Length > 0)
{
rfe.facet = current.facet;
}

if (rfe.operation.Length == 0 && current.operation.Length > 0)
{
rfe.operation = current.operation;
}

// Called after fixing id, facet and operation.
exceptionMessage = rfe.ToString();
exceptionMessage = RequestFailedException.createMessage(rfe.GetType().Name, id, facet, operation);

if (current.requestId != 0)
{
ostr.writeByte((byte)replyStatus);
Identity.ice_write(ostr, rfe.id);
Identity.ice_write(ostr, id);

if (rfe.facet.Length == 0)
if (facet.Length == 0)
{
ostr.writeStringSeq([]);
}
else
{
ostr.writeStringSeq([rfe.facet]);
ostr.writeStringSeq([facet]);
}

ostr.writeString(rfe.operation);
ostr.writeString(operation);
}
break;

Expand Down
Loading

0 comments on commit 0831ab1

Please sign in to comment.