refactor: normalize log and exception messages (#19)

Author: andrei-cristea

CHANGELOG.md

@@ -6,7 +6,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## [Unreleased]
 ### Changed
+- Normalized logging and error messages using Keyple coding standards.
 - Migrated the CI pipeline from Jenkins to GitHub Actions.
+### Upgraded
+- `keyple-util-java-lib` from `2.4.0` to `2.4.1` (source code not impacted)
+- `slf4j-api` from `1.7.32` to `1.7.36` (`compileOnly`)
 
 ## [2.5.1] - 2024-09-19
 ### Fixed

build.gradle.kts

@@ -14,10 +14,11 @@ plugins {
 ///////////////////////////////////////////////////////////////////////////////
 
 dependencies {
-  implementation("org.eclipse.keyple:keyple-util-java-lib:2.4.0")
+  implementation("org.eclipse.keyple:keyple-util-java-lib:2.4.1")
   implementation("com.google.code.gson:gson:2.10.1")
-  implementation("org.slf4j:slf4j-api:1.7.32")
-  testImplementation("org.slf4j:slf4j-simple:1.7.32")
+  compileOnly("org.slf4j:slf4j-api:1.7.36")
+
+  testImplementation("org.slf4j:slf4j-simple:1.7.36")
   testImplementation(platform("org.junit:junit-bom:5.10.2"))
   testImplementation("org.junit.jupiter:junit-jupiter")
   testImplementation("org.junit.vintage:junit-vintage-engine")

src/main/java/org/eclipse/keyple/distributed/AbstractNodeAdapter.java

@@ -113,11 +113,7 @@ final void closeSessionSilently(String sessionId) {
     try {
       closeSession(sessionId);
     } catch (RuntimeException e) {
-      logger.error(
-          "Error during the silent closing of node's session [{}]: {}",
-          sessionId,
-          e.getMessage(),
-          e);
+      logger.error("Failed to close node session safely [sessionId={}]", sessionId, e);
     }
   }
 
@@ -319,10 +315,7 @@ synchronized void waitForState(SessionManagerState... targetStates) {
         }
         timeoutOccurred();
       } catch (InterruptedException e) {
-        logger.error(
-            "Unexpected interruption of the task associated with the node's session {}",
-            sessionId,
-            e);
+        logger.error("Unexpected node session task interruption [sessionId={}]", sessionId, e);
         Thread.currentThread().interrupt();
       }
     }
@@ -350,9 +343,12 @@ void checkState(SessionManagerState... targetStates) {
         }
       }
       throw new IllegalStateException(
-          String.format(
-              "The status of the node's session manager [%s] should have been one of %s, but is currently [%s]",
-              sessionId, Arrays.toString(targetStates), state));
+          "The status of the node's session manager '"
+              + sessionId
+              + "' should have been one of "
+              + Arrays.toString(targetStates)
+              + ", but is currently "
+              + state);
     }
 
     /**
@@ -363,11 +359,9 @@ void checkState(SessionManagerState... targetStates) {
      */
     void timeoutOccurred() {
       state = SessionManagerState.ABORTED_SESSION;
-      logger.error(
-          "Timeout occurs for the task associated with the node's session [{}]", sessionId);
+      logger.error("Node session task timed out [sessionId={}]", sessionId);
       throw new NodeCommunicationException(
-          String.format(
-              "Timeout occurs for the task associated with the node's session [%s]", sessionId));
+          "Timeout occurs for the task associated with the node's session '" + sessionId + "'");
     }
   }
 }

src/main/java/org/eclipse/keyple/distributed/AsyncNodeClientAdapter.java

@@ -176,7 +176,9 @@ public void onError(String sessionId, Throwable error) {
   private SessionManager getManagerForEndpoint(String sessionId) {
     SessionManager manager = sessionManagers.get(sessionId);
     if (manager == null) {
-      logger.warn("Node's session not found [{}]. It was maybe closed due to a timeout", sessionId);
+      logger.warn(
+          "Node session not found. It was maybe closed due to a timeout event [sessionId={}]",
+          sessionId);
     }
     return manager;
   }

src/main/java/org/eclipse/keyple/distributed/AsyncNodeServerAdapter.java

@@ -75,8 +75,7 @@ void sendMessage(MessageDto message) {
   private SessionManager getManagerForHandler(String sessionId) {
     SessionManager manager = sessionManagers.get(sessionId);
     if (manager == null) {
-      throw new IllegalStateException(
-          String.format("The node's session [%s] is closed", sessionId));
+      throw new IllegalStateException("The node's session '" + sessionId + "' is closed");
     }
     return manager;
   }

src/main/java/org/eclipse/keyple/distributed/MessageDto.java

@@ -398,4 +398,38 @@ public final MessageDto setBody(String body) {
     this.body = body;
     return this;
   }
+
+  /**
+   * {@inheritDoc}
+   *
+   * @since 2.5.2
+   */
+  @Override
+  public String toString() {
+    return "MessageDto{"
+        + "apiLevel="
+        + apiLevel
+        + ", sessionId='"
+        + sessionId
+        + '\''
+        + ", action='"
+        + action
+        + '\''
+        + ", clientNodeId='"
+        + clientNodeId
+        + '\''
+        + ", serverNodeId='"
+        + serverNodeId
+        + '\''
+        + ", localReaderName='"
+        + localReaderName
+        + '\''
+        + ", remoteReaderName='"
+        + remoteReaderName
+        + '\''
+        + ", body='"
+        + body
+        + '\''
+        + '}';
+  }
 }

src/main/java/org/eclipse/keyple/distributed/SyncNodeClientAdapter.java

@@ -91,9 +91,9 @@ MessageDto sendRequest(MessageDto message) {
       return response;
     } else {
       throw new IllegalStateException(
-          String.format(
-              "The list returned by the client endpoint should have contained a single element but contains %s elements",
-              responses.size()));
+          "The list returned by the client endpoint should have contained a single element but contains "
+              + responses.size()
+              + " elements");
     }
   }
 
@@ -246,7 +246,7 @@ public void run() {
           try {
             Thread.sleep(requestFrequencyMillis);
           } catch (InterruptedException e) {
-            logger.error("Unexpected interruption of thread {}", getName(), e);
+            logger.error("Unexpected thread interruption [thread={}]", getName(), e);
             Thread.currentThread().interrupt();
           }
         }
@@ -267,7 +267,10 @@ public void run() {
     private class EventObserverUncaughtExceptionHandler implements Thread.UncaughtExceptionHandler {
       @Override
       public void uncaughtException(Thread t, Throwable e) {
-        logger.error("Interruption of thread {} caused by an unhandled exception", t.getName(), e);
+        logger.error(
+            "Unexpected thread interruption caused by an unhandled exception [thread={}]",
+            t.getName(),
+            e);
       }
     }
 
@@ -306,7 +309,7 @@ private List<MessageDto> retryRequest() {
         try {
           timer = timer1 + timer2;
           Thread.sleep(timer);
-          logger.info("Retry to send request after {} seconds...", timer / 1000);
+          logger.info("Retrying to send request after {} seconds", timer / 1000);
           responses = sendRequestSilently();
           if (responses != null) {
             logger.info("Server connection retrieved");
@@ -316,7 +319,8 @@ private List<MessageDto> retryRequest() {
             timer2 = timer;
           }
         } catch (InterruptedException e) {
-          logger.error("Unexpected interruption of thread {}", Thread.currentThread().getName(), e);
+          logger.error(
+              "Unexpected thread interruption [thread={}]", Thread.currentThread().getName(), e);
           Thread.currentThread().interrupt();
         }
       }

src/main/java/org/eclipse/keyple/distributed/SyncNodeServerAdapter.java

@@ -196,8 +196,7 @@ private void checkEventManagers(
     // Check the current client node ID.
     if (unusedClientNodeIds.contains(clientNodeId)) {
       throw new IllegalStateException(
-          String.format(
-              "Client node ID [%s] removed because not used for at least 1 day", clientNodeId));
+          "Client node ID '" + clientNodeId + "' removed because not used for at least 1 day");
     }
   }
 
@@ -436,10 +435,7 @@ private synchronized void waitAtMost(int maxAwaitingTimeMillis) {
           wait(maxAwaitingTimeMillis);
         }
       } catch (InterruptedException e) {
-        logger.error(
-            "Unexpected interruption of the task associated with the node's id {}",
-            clientNodeId,
-            e);
+        logger.error("Unexpected node task interruption [clientNodeId={}]", clientNodeId, e);
         Thread.currentThread().interrupt();
       }
     }