Refactor Java Local Exceptions #2590

InsertCreativityHere · 2024-07-31T14:45:37Z

This PR finishes our refactoring of local exceptions by doing it in Java.
For a list of which exceptions changed to what, see this table from the original PR.

Along the way, I cross-checked my changes with the other languages.
While doing so, I noticed some bugs, inconsistencies, typos, etc. that seemed small and related enough to just fix in this PR.

bernardnormier · 2024-08-05T21:55:13Z

java/src/Ice/src/main/java/com/zeroc/Ice/DatagramLimitException.java

-
-  public DatagramLimitException(Throwable cause) {
-    super(cause);
+    this("");


This doesn't look correct.

The C# version has a single ctor:

public DatagramLimitException() : base(message: "Datagram limit exceeded.", innerException: null) { }

This doesn't look correct.

I fixed the message

The C# version has a single ctor

This is because of a difference in the UdpTransceiver.
In C#, we just use the default message:
throw new Ice.DatagramLimitException();
but in Java, we also include the packetSize in the message:
throw new com.zeroc.Ice.DatagramLimitException("message size of " + buf.size() + " exceeds the maximum packet size of " + packetSize);

In C++, we don't include this extra information:
throw DatagramLimitException(__FILE__, __LINE__);
So, Java is the odd-one-out here.
But this information seems useful to have... If you exceeded a limit, it's good to know what the limit was, right?

bernardnormier · 2024-08-05T21:56:31Z

java/src/Ice/src/main/java/com/zeroc/Ice/InvocationCanceledException.java

-    super(cause);
+public final class InvocationCanceledException extends LocalException {
+  public InvocationCanceledException() {
+    super("invocation canceled");


Should be consistent with the InvocationTimeoutException message.

Fixed by making it a proper sentence.

bernardnormier · 2024-08-05T21:57:02Z

java/src/Ice/src/main/java/com/zeroc/Ice/MarshalException.java

-  }
-
+/** This exception is raised for errors during marshaling or unmarshaling. */
+public final class MarshalException extends ProtocolException {
  public MarshalException(String reason) {


The string parameter should be renamed 'message'.

Good catch, fixed.

bernardnormier · 2024-08-05T22:04:43Z

java/src/Ice/src/main/java/com/zeroc/Ice/LocalException.java

-/** Base class for all Ice run-time exceptions. */
-public abstract class LocalException extends Exception {
+/** Base class for Ice run-time exceptions. */
+public class LocalException extends Exception {
  public LocalException() {}


I would remove this parameterless constructor. The caller (including derived classes) must provide a message and/or a cause.

Removed. Now all constructors require a String message.

bernardnormier · 2024-08-05T22:05:48Z

java/src/Ice/src/main/java/com/zeroc/Ice/LocalException.java

+  public LocalException(String message) {
+    super(message);
+  }
+
  public LocalException(Throwable cause) {


Not sure we need this cause-only constructor.

Also removed. Now all constructors require a String message.

bernardnormier · 2024-08-05T22:11:09Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownException.java

    this.unknown = unknown;
  }

  public UnknownException(String unknown, Throwable cause) {
-    super(cause);
+    super("unknown exception with type ID '" + unknown + "'", cause);


This is bogus.

Fixed, after discussing what this parameter actually holds.

bernardnormier · 2024-08-05T22:11:19Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownException.java

  public UnknownException(Throwable cause) {
    super(cause);
    this.unknown = "";
  }

  public UnknownException(String unknown) {
+    super("unknown exception with type ID '" + unknown + "'");


This is bogus.

Fixed, after discussing what this parameter actually holds.

bernardnormier · 2024-08-05T22:11:45Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownLocalException.java

-    super(cause);
-  }
-
+public final class UnknownLocalException extends UnknownException {
  public UnknownLocalException(String unknown) {


Should be message, not unknown.

Fixed, we only have a single constructor, which only takes a String message.

bernardnormier · 2024-08-05T22:12:09Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownUserException.java

-    super(cause);
-  }
-
+public final class UnknownUserException extends UnknownException {
  public UnknownUserException(String unknown) {


Should be message, not unknown.

Also, please don't provide a 'cause' constructor.

Fixed, we only have a single constructor, which only takes a String message.

bernardnormier · 2024-08-05T22:17:08Z

java/src/Ice/src/main/java/com/zeroc/Ice/RequestFailedException.java

-    this.id = new Identity();
-    this.facet = "";
-    this.operation = "";
+    this(new Identity(), "", "");


This won't produce a good message.

pepone · 2024-08-06T07:23:28Z

java/src/Ice/src/main/java/com/zeroc/Ice/DNSException.java

+/** This exception indicates a DNS problem. */
+public final class DNSException extends SyscallException {
+  public DNSException(String host) {
+    super("Cannot resolve host '" + host + "'");
    this.host = host;


We remove the host field in C#

Removed in Java.

pepone · 2024-08-06T07:27:38Z

java/src/Ice/src/main/java/com/zeroc/Ice/DNSException.java

+/** This exception indicates a DNS problem. */
+public final class DNSException extends SyscallException {
+  public DNSException(String host) {
+    super("Cannot resolve host '" + host + "'");


I think we can do:

Suggested change

super("Cannot resolve host '" + host + "'");

this(host, null);

Possibly surprising, but calling super(message, null) is slightly different than super(message). since these constructors do different things, and the default value for cause isn't actually null, it's self, which acts as a special sentinel value.

My understanding is that explicitly setting a null cause is used to indicate that something caused it, but for some reason it's unknown. Which is not the case here.

It probably doesn't matter here... this is a niche exception, but also I find the duplication to be pretty small anyways.

pepone · 2024-08-06T07:30:57Z

java/src/Ice/src/main/java/com/zeroc/Ice/DNSException.java

+    super("Cannot resolve host '" + host + "'");
    this.host = host;


Can't we call the other constructor here?

Suggested change

super("Cannot resolve host '" + host + "'");

this.host = host;

this(host, null);

Possibly surprising, but calling super(message, null) is slightly different than super(message). since these constructors do different things, and the default value for cause isn't actually null, it's self, which acts as a special sentinel value.

My understanding is that explicitly setting a null cause is used to indicate that something caused it, but for some reason it's unknown. Which is not the case here.

pepone · 2024-08-06T07:33:00Z

java/src/Ice/src/main/java/com/zeroc/Ice/DatagramLimitException.java

-
-  public DatagramLimitException(Throwable cause) {
-    super(cause);
+    this("");


The C# version has a single ctor:

public DatagramLimitException() : base(message: "Datagram limit exceeded.", innerException: null) { }

pepone · 2024-08-06T07:34:10Z

java/src/Ice/src/main/java/com/zeroc/Ice/Exception.java

+  public Exception(String message) {
+    super(message);
+  }
+
  public Exception(Throwable cause) {


Do we need this constructor?

No, I deleted it.

I thought that UserException derived from this class, but apparently it doesn't in Java (because of checked vs unchecked exceptions).

pepone · 2024-08-06T07:40:57Z

java/src/Ice/src/main/java/com/zeroc/Ice/RequestFailedException.java

@@ -22,26 +9,11 @@
 */
 public class RequestFailedException extends LocalException {
  public RequestFailedException() {
-    this.id = new Identity();


This constructor should not call the other.

Yes, my bad.
Fixed.

pepone · 2024-08-06T07:42:21Z

java/src/Ice/src/main/java/com/zeroc/Ice/RequestFailedException.java

-
-  public RequestFailedException(Identity id, String facet, String operation, Throwable cause) {
-    super(cause);
+    super(createMessage("RequestFailedException", id, facet, operation));


The derived classes must pass the typeName, we don't want to use RequestFailedException in the message.

Fixed, I forgot to add a typename parameter to this constructor.

pepone · 2024-08-06T07:47:08Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownException.java

-  public UnknownException() {
-    this.unknown = "";
-  }
-
  public UnknownException(Throwable cause) {


Other mappings have a single constructor with the message.

Java makes frequent use of the constructor, mostly in cases where we encounter a competely unknown exception, that we know nothing else about, and just wrap in an UnknownException.
Looking at the corresponding code in C#, we don't handle these exceptions, and just let them throw upwards.
But, some of those places have comments which suggests the Java behavior is more correct.

For example in ConnectionI, java has an extra catch for truly unknown exceptions:

} catch (LocalException ex) { dispatchException(ex, requestCount); } catch (RuntimeException | java.lang.Error ex) { // A runtime exception or an error was thrown outside of servant code (i.e., by Ice code). // Note that this code does NOT send a response to the client. var uex = new UnknownException(ex);

and in C#, we just have a TODO.

catch (LocalException ex) // TODO: catch all exceptions { dispatchException(ex, requestCount); }

Or in LoggerAdminLoggerI, java has an extra else to handle truly unknown exceptions,

if (ex instanceof com.zeroc.Ice.CommunicatorDestroyedException) { // Expected if there are outstanding calls during communicator destruction. } else if (ex instanceof com.zeroc.Ice.LocalException) { _loggerAdmin.deadRemoteLogger( p, _localLogger, (com.zeroc.Ice.LocalException) ex, "log"); } else { _loggerAdmin.deadRemoteLogger( p, _localLogger, new com.zeroc.Ice.UnknownException(ex), "log"); }

whereas C# looks like it just silently ignores unknown errors here:

catch (AggregateException ae) { if (ae.InnerException is Ice.CommunicatorDestroyedException) { // expected if there are outstanding calls during communicator destruction } if (ae.InnerException is Ice.LocalException) { _loggerAdmin.deadRemoteLogger(p, _localLogger, (Ice.LocalException)ae.InnerException, "log"); } }

We should definitely remove Throwable from all UnknownException constructors and fix any code that use a UnknownException Throwable constructor.

The name Unknown is confusing but we can't change it. An UnknownException is not unknown. It's a very specific exception that means a dispatch failed with an "other" exception (other than local exception or user exception).
Whoever wrote the Java code you're showing above was unfortunately confused by the name.

We also don't want users to think "I'll throw an UnknownException that wraps some random exception - Ice likes that and will send something sensible to the client" because that's not the case.

pepone · 2024-08-06T07:56:20Z

java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyOutgoingAsyncBaseI.java

-
-            ex.unknown = unknown;
-            throw ex;
+            String unknownId = is.readString();


We read the message:

String message = ...

Fixed, I had misunderstood what this unknown field truly held.

pepone · 2024-08-06T08:12:15Z

matlab/lib/+IceInternal/EncapsDecoder11.m

@@ -348,6 +348,7 @@ function skipSlice(obj)
                        catch e
                            reason = sprintf('constructor failed for type %s with compact id %d', ...
                                eval(sprintf('IceCompactId.TypeId_%d.typeId', compactId)), compactId);
+                            % TODO does this type still exist in MATLAB?
                            ex = Ice.NoValueFactoryException('', reason, reason, '');


Seems this is the only occurrence, better create a separate issue to fix it.

@bernardnormier beat me to opening an issue, and already opened a PR to fix this: #2626

InsertCreativityHere · 2024-07-31T14:46:11Z

cpp/src/slice2js/Gen.cpp

@@ -2581,7 +2581,7 @@ Slice::Gen::TypeScriptVisitor::visitInterfaceDefStart(const InterfaceDefPtr& p)
    _out << nl << " * @param communicator - The communicator for the new proxy.";
    _out << nl << " * @param proxyString - The string representation of the proxy.";
    _out << nl << " * @returns The new " << prx << " proxy.";
-    _out << nl << " * @throws ProxyParseException - Thrown if the proxyString is not a valid proxy string.";
+    _out << nl << " * @throws ParseException - Thrown if the proxyString is not a valid proxy string.";


ProxyParseException was replaced with ParseException

InsertCreativityHere · 2024-07-31T14:47:02Z

csharp/src/Ice/Internal/ReferenceFactory.cs

@@ -203,7 +203,7 @@ public Reference create(string s, string propertyPrefix)
                    end = Ice.UtilInternal.StringUtil.checkQuote(s, beg);
                    if (end == -1)
                    {
-                        throw new ParseException($"mismatched quotes around value for '{option}' option in proxy string '{s}'");
+                        throw new ParseException($"mismatched quotes around value for {option} option in proxy string '{s}'");


Everywhere else in this file, we don't put quotes around the {option} (ex: -c, -t, -h, ...)

InsertCreativityHere · 2024-07-31T14:47:18Z

csharp/src/Ice/Internal/TcpEndpointI.cs

@@ -233,12 +233,12 @@ protected override bool checkOption(string option, string argument, string endpo
                        _timeout = int.Parse(argument, CultureInfo.InvariantCulture);
                        if (_timeout < 1)
                        {
-                            throw new ParseException($"invalid timeout value '{argument}' in endpoint {endpoint}");
+                            throw new ParseException($"invalid timeout value '{argument}' in endpoint '{endpoint}'");


In every other place in this PR, we always wrapped the {endpoint} in single quotes

InsertCreativityHere · 2024-07-31T14:47:37Z

csharp/src/Ice/Internal/UdpEndpointI.cs

@@ -337,7 +337,7 @@ protected override bool checkOption(string option, string argument, string endpo
        {
            if (argument == null)
            {
-                throw new ParseException($"unexpected argument '{argument}' provided for {option} option in endpoint '{endpoint}'");
+                throw new ParseException($"no argument provided for {option} option in endpoint '{endpoint}'");


Looked like an over-zealous copy/paste.
This is what the message used to be, and continues to be in other place we test for a null argument.
Also it's kind of weird to check argument == null, then try to print it...

InsertCreativityHere · 2024-07-31T14:47:51Z

csharp/src/Ice/ObjectAdapter.cs

@@ -1441,7 +1441,7 @@ private List<EndpointI> parseEndpoints(string endpts, bool oaEndpoints)
            EndpointI endp = _instance.endpointFactoryManager().create(s, oaEndpoints);
            if (endp is null)
            {
-                throw new ParseException($"invalid object adapter endpoint {s}'");
+                throw new ParseException($"invalid object adapter endpoint '{s}'");


mismatched single quotes.

Sounds like this should also be a full sentence, but I'm going to leave it alone for now.

InsertCreativityHere · 2024-08-06T21:39:14Z

java/src/Ice/src/main/java/com/zeroc/Ice/RequestFailedException.java

@@ -22,26 +9,11 @@
 */
 public class RequestFailedException extends LocalException {
  public RequestFailedException() {
-    this.id = new Identity();


Yes, my bad.
Fixed.

InsertCreativityHere · 2024-08-06T21:39:16Z

java/src/Ice/src/main/java/com/zeroc/Ice/RequestFailedException.java

-
-  public RequestFailedException(Identity id, String facet, String operation, Throwable cause) {
-    super(cause);
+    super(createMessage("RequestFailedException", id, facet, operation));


Fixed, I forgot to add a typename parameter to this constructor.

InsertCreativityHere · 2024-08-06T21:39:19Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownException.java

-  public UnknownException() {
-    this.unknown = "";
-  }
-
  public UnknownException(Throwable cause) {


Java makes frequent use of the constructor, mostly in cases where we encounter a competely unknown exception, that we know nothing else about, and just wrap in an UnknownException.
Looking at the corresponding code in C#, we don't handle these exceptions, and just let them throw upwards.
But, some of those places have comments which suggests the Java behavior is more correct.

For example in ConnectionI, java has an extra catch for truly unknown exceptions:

} catch (LocalException ex) { dispatchException(ex, requestCount); } catch (RuntimeException | java.lang.Error ex) { // A runtime exception or an error was thrown outside of servant code (i.e., by Ice code). // Note that this code does NOT send a response to the client. var uex = new UnknownException(ex);

and in C#, we just have a TODO.

catch (LocalException ex) // TODO: catch all exceptions { dispatchException(ex, requestCount); }

Or in LoggerAdminLoggerI, java has an extra else to handle truly unknown exceptions,

if (ex instanceof com.zeroc.Ice.CommunicatorDestroyedException) { // Expected if there are outstanding calls during communicator destruction. } else if (ex instanceof com.zeroc.Ice.LocalException) { _loggerAdmin.deadRemoteLogger( p, _localLogger, (com.zeroc.Ice.LocalException) ex, "log"); } else { _loggerAdmin.deadRemoteLogger( p, _localLogger, new com.zeroc.Ice.UnknownException(ex), "log"); }

whereas C# looks like it just silently ignores unknown errors here:

catch (AggregateException ae) { if (ae.InnerException is Ice.CommunicatorDestroyedException) { // expected if there are outstanding calls during communicator destruction } if (ae.InnerException is Ice.LocalException) { _loggerAdmin.deadRemoteLogger(p, _localLogger, (Ice.LocalException)ae.InnerException, "log"); } }

InsertCreativityHere · 2024-08-06T21:39:21Z

java/src/Ice/src/main/java/com/zeroc/IceInternal/ProxyOutgoingAsyncBaseI.java

-
-            ex.unknown = unknown;
-            throw ex;
+            String unknownId = is.readString();


Fixed, I had misunderstood what this unknown field truly held.

InsertCreativityHere · 2024-08-06T21:39:23Z

matlab/lib/+IceInternal/EncapsDecoder11.m

@@ -348,6 +348,7 @@ function skipSlice(obj)
                        catch e
                            reason = sprintf('constructor failed for type %s with compact id %d', ...
                                eval(sprintf('IceCompactId.TypeId_%d.typeId', compactId)), compactId);
+                            % TODO does this type still exist in MATLAB?
                            ex = Ice.NoValueFactoryException('', reason, reason, '');


@bernardnormier beat me to opening an issue, and already opened a PR to fix this: #2626

bernardnormier · 2024-08-06T23:24:41Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownException.java

-  public UnknownException() {
-    this.unknown = "";
-  }
-
  public UnknownException(Throwable cause) {


We should definitely remove Throwable from all UnknownException constructors and fix any code that use a UnknownException Throwable constructor.

The name Unknown is confusing but we can't change it. An UnknownException is not unknown. It's a very specific exception that means a dispatch failed with an "other" exception (other than local exception or user exception).
Whoever wrote the Java code you're showing above was unfortunately confused by the name.

We also don't want users to think "I'll throw an UnknownException that wraps some random exception - Ice likes that and will send something sensible to the client" because that's not the case.

bernardnormier · 2024-08-06T23:28:06Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownUserException.java

- * exceptions declared in the <code>throws</code> clause can be raised.
+ * The dispatch returned a {@link UserException} that was not declared in the operation's exception
+ * specification (the <code>throws</code> clause). This is necessary in order to not violate the
+ * contract established by an operation's signature: Only local exceptions and user exceptions


This doc comment is bogus.

Why? Do we allow arbitrary user exceptions to be thrown now?

bernardnormier · 2024-08-06T23:28:22Z

java/src/Ice/src/main/java/com/zeroc/Ice/UnknownUserException.java

-
-  public UnknownUserException(String unknown, Throwable cause) {
-    super(unknown, cause);
+  public UnknownUserException(String message, Throwable cause) {


Please no Throwable.

I removed this.
In Java, we used to store the actual UserException in this field, so we could put it's stack trace in the expectedDetails.
Removing this does means that the details will be less... detailed now.

bernardnormier · 2024-08-06T23:30:38Z

cpp/src/Ice/Initialize.cpp

@@ -179,7 +179,7 @@ Ice::ThreadHookPlugin::ThreadHookPlugin(
 {
    if (communicator == nullptr)
    {
-        throw PluginInitializationException(__FILE__, __LINE__, "Communicator cannot be null");
+        throw PluginInitializationException(__FILE__, __LINE__, "Communicator cannot be null.");


Yes, please revert.
In C++, the style we adopted is lowercase and no period. It should actually be communicator here.

I'm just trying to keep all the error messages consistent across languages.

We don't want to do that. The goal is for error messages in language X to be consistent with error message in language X. Not adopt our own cross-language Ice style.

bernardnormier · 2024-08-06T23:31:38Z

cpp/src/Ice/ObjectAdapterI.cpp

@@ -934,7 +934,7 @@ Ice::ObjectAdapterI::initialize(optional<RouterPrx> router)
        //
        if (router == nullopt && noProps)
        {
-            throw InitializationException(__FILE__, __LINE__, "object adapter `" + _name + "' requires configuration");
+            throw InitializationException(__FILE__, __LINE__, "Object adapter `" + _name + "' requires configuration.");


Please revert all these updates.

Alright, now that I have a bearing for what the intent is, I'll revert/fix my changes as necessary.
Unfortunately, we're very inconsistent about which message style we use across our languages, so extrapolated the wrong idea.

One day we should consider a cleanup pass to fix all these messages...

I fixed all my changes to the messages, not by reverting them, but by changing them to be correct in each language.
Since most of them didn't follow the guidelines you laid out...

bernardnormier · 2024-08-06T23:32:31Z

csharp/src/Ice/ConnectionI.cs

@@ -171,7 +171,7 @@ public void close(ConnectionClose mode)
                setState(
                    StateClosing,
                    new ConnectionClosedException(
-                        "The connection was closed gracefully by the application.",
+                        "Connection closed gracefully by the application.",


The preferred style in C# is a full sentence. So I would keep "Ther connection was ..." and fix the message below.

bernardnormier · 2024-08-06T23:36:04Z

csharp/src/Ice/PluginManagerI.cs

@@ -44,7 +44,7 @@ public void initializePlugins()
    {
        if (_initialized)
        {
-            throw new InitializationException("Plug-ins already initialized.");
+            throw new InitializationException("plug-ins already initialized");


We should follow each programming language best practices. In C#, it's full sentences, starting with a capital letter and ending with a period. Please revert this update.

bernardnormier · 2024-08-06T23:42:01Z

java/src/Ice/src/main/java/com/zeroc/Ice/Current.java

@@ -256,15 +254,15 @@ private OutgoingResponse createOutgoingResponseCore(Throwable exc) {
    } else if (exc instanceof UnknownLocalException ex) {
      exceptionId = ex.ice_id();
      replyStatus = ReplyStatus.UnknownLocalException;
-      unknownExceptionMessage = ex.unknown;
+      unknownExceptionMessage = ex.toString();


This is not correct. The message of an UnknownXxxException is returned by getMessage, not toString.

Also fixed.

bernardnormier · 2024-08-06T23:42:22Z

java/src/Ice/src/main/java/com/zeroc/Ice/Current.java

@@ -285,13 +283,17 @@ private OutgoingResponse createOutgoingResponseCore(Throwable exc) {
            || replyStatus == ReplyStatus.UnknownException)) {
      ostr.writeByte(replyStatus.value());
      if (unknownExceptionMessage == null) {
-        unknownExceptionMessage = "Dispatch failed with " + exceptionId + ": " + exc.getMessage();
+        unknownExceptionMessage = "Dispatch failed with " + exc.toString();


That's correct.

bernardnormier · 2024-08-06T23:45:59Z

java/src/Ice/src/main/java/com/zeroc/Ice/Current.java

-      if (rfe.id.name.isEmpty()) {
-        rfe.id = id;
-        rfe.facet = facet;
+      Identity id = rfe.id;


I find defining local variables that hide fields pretty confusing and unexpected, especially in a big method like this one.

I'm open to changing it, but this is what we're doing in every language, so I'd like to change it in all of them.
See #2345

In other languages, this code is not in Current, but in a separate class. So id/facet/operation don't hide fields.

Fair enough, I added a prefix in Java to disambiguate them.

pepone · 2024-08-07T07:12:09Z

java/src/Ice/src/main/java/com/zeroc/Ice/SocketException.java

@@ -5,7 +5,7 @@
 /** This exception indicates socket errors. */
 public class SocketException extends SyscallException {
  public SocketException() {
-    super();
+    super((String) null);


What is the difference between super() and super((String)null)?

The parameterless constructor was removed from SyscallException, and Java doesn't support default parameters, so here we explicitly pass null to say that we don't have any useful message.

But SyscallException has 2 constructors which only take 1 parameter:
SyscallException(Throwable cause) and SyscallException(String message)
so passing null is ambiguous, so we have to do this awkward looking cast to let the compiler know which constructor we want to call.

InsertCreativityHere · 2024-08-07T17:46:40Z

@bernardnormier

We should definitely remove Throwable from all UnknownException constructors and fix any code that use a UnknownException Throwable constructor.

I removed it from UnknownUserException and UnknownLocalException, but I'm going to do the remainder in a separate PR.
The Java mapping wraps all kinds of (seemingly) random exceptions in UnknownException, and untangling those feels unrelated to consolidating the LocalExceptions themselves.

bernardnormier · 2024-08-07T18:06:03Z

java/src/Ice/src/main/java/com/zeroc/Ice/Current.java

-      if (rfe.operation.isEmpty()) {
-        rfe.operation = operation;
-      }
+      String operation = rfe.operation.isEmpty() ? this.operation : rfe.operation;


operation also shadows a field

…-ice/ice#2590)

InsertCreativityHere added 5 commits July 30, 2024 16:49

Transformed all the exception types.

ae2b5a5

Bug fixes.

0411174

More noise from the formatter.

4b75114

Final checkpoint of phase 1.

0ad3b33

TEMP

8dedac2

InsertCreativityHere force-pushed the java-local-exceptions branch from efecea1 to 8dedac2 Compare July 31, 2024 18:59

InsertCreativityHere added 5 commits July 31, 2024 18:10

We've reached another checkpoint.

2be9b84

Simplified the exception definitions themselves.

2334e36

Deleted log file which snuck into a commit.

a1f1210

Round of bugfixes/tweaks.

8cbc851

Finished synchronizing comments.

4a1c429

InsertCreativityHere force-pushed the java-local-exceptions branch from 3257b61 to 4a1c429 Compare August 5, 2024 20:52

InsertCreativityHere added 3 commits August 5, 2024 17:14

Fixed a python test.

e693242

Merge branch 'main' into java-local-exceptions

516c183

Last couple consistency changes.

60efc65

InsertCreativityHere marked this pull request as ready for review August 5, 2024 21:44

InsertCreativityHere requested review from pepone, bernardnormier and externl August 5, 2024 21:44

bernardnormier reviewed Aug 5, 2024

View reviewed changes

pepone reviewed Aug 6, 2024

View reviewed changes

Fixed review comments.

927a4b9

InsertCreativityHere commented Aug 6, 2024

View reviewed changes

InsertCreativityHere requested review from bernardnormier and pepone August 6, 2024 21:40

bernardnormier reviewed Aug 6, 2024

View reviewed changes

pepone approved these changes Aug 7, 2024

View reviewed changes

externl removed their request for review August 7, 2024 14:23

Fixed wording changes in other languages.

ca23ce9

bernardnormier self-requested a review August 7, 2024 15:47

InsertCreativityHere added 2 commits August 7, 2024 11:47

Current fixes.

f1fe969

Fix 'UnknownUserException' message/constructors.

7f0393b

InsertCreativityHere added 2 commits August 7, 2024 13:49

Tweaked message of 'UnknownUserException' to be more clear.

ba9bb69

Fix exception messages in 'servantLocator' test.

a96f040

bernardnormier approved these changes Aug 7, 2024

View reviewed changes

Renamed 'operation' variable too.

ce5dc6d

InsertCreativityHere merged commit b1277ef into zeroc-ice:main Aug 7, 2024
19 checks passed

InsertCreativityHere added a commit to InsertCreativityHere/compiler-comparison that referenced this pull request Jan 1, 2025

HEAD:(zeroc-ice/ice@b1277ef447) Refactor Java Local Exceptions (zeroc…

76895d7

…-ice/ice#2590)

	super("Cannot resolve host '" + host + "'");
	this(host, null);

		super("Cannot resolve host '" + host + "'");
		this.host = host;

Refactor Java Local Exceptions #2590

Refactor Java Local Exceptions #2590

Conversation

InsertCreativityHere commented Jul 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

InsertCreativityHere commented Jul 31, 2024 •

edited

Loading