Itiviti · nhaquet2 · Oct 5, 2022 · Oct 5, 2022 · Oct 5, 2022
diff --git a/src/main/java/com/ullink/slack/review/gerrit/ReviewRequestCleanupTask.java b/src/main/java/com/ullink/slack/review/gerrit/ReviewRequestCleanupTask.java
@@ -2,13 +2,17 @@
 
 import java.util.Collection;
 import java.util.concurrent.ExecutorService;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import jobs.DeleteMessageJob;
 import jobs.RefreshMessageJob;
+import com.ullink.slack.review.HttpHelper;
 import com.ullink.slack.review.gerrit.reviewrequests.ReviewRequestService;
 import com.ullink.slack.simpleslackapi.SlackSession;
 
 public class ReviewRequestCleanupTask implements Runnable
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(ReviewRequestCleanupTask.class);
 
     private ReviewRequestService    reviewRequestService;
     private GerritChangeInfoService gerritChangeInfoService;
@@ -41,8 +45,10 @@ public void run()
                 {
                     executorService.submit(new RefreshMessageJob(changeId, session, reviewRequestService, gerritChangeInfoService, changeInfoDecorator));
                 }
-            } catch (RuntimeException e) {
-                // DO NOTHING, error was logged
+            }
+            catch (Throwable t)
+            {
+                LOGGER.error("Unexpected error, t");
             }
         }
     }

diff --git a/src/main/java/commands/PublishReviewCommandProcessor.java b/src/main/java/commands/PublishReviewCommandProcessor.java
@@ -21,6 +21,8 @@
 import com.ullink.slack.simpleslackapi.SlackChannel;
 import com.ullink.slack.simpleslackapi.SlackSession;
 import com.ullink.slack.simpleslackapi.events.SlackMessagePosted;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Singleton
 public class PublishReviewCommandProcessor implements SlackBotCommandProcessor
@@ -36,6 +38,8 @@ public class PublishReviewCommandProcessor implements SlackBotCommandProcessor
     @Inject
     private ChangeInfoFormatter changeInfoDecorator;
 
+    private static final Logger LOGGER       = LoggerFactory.getLogger(PublishReviewCommandProcessor.class);
+
     private static final String COMMAND = "!publishreview";
     private final Pattern PUBLISH_REVIEW_PATTERN = Pattern.compile(COMMAND + SPACES
         + "(" + CHANNEL + ")" + SPACES
@@ -56,10 +60,14 @@ public boolean process(String command, SlackMessagePosted event, SlackSession se
             String changeId = matcher.group(2);
             String comment = matcher.group(4);
             SlackChannel channel = session.findChannelByName(channelNameToPublish);
-            if (channel != null)
+            if (channel != null && changeId != null && !changeId.trim().isEmpty())
             {
                 executor.execute(new PublishMessageJob(channelNameToPublish, event.getChannel(), changeId.trim(), comment, session, reviewRequestService, subscriptionService, gerritChangeInfoService, changeInfoDecorator));
             }
+            else
+            {
+                LOGGER.error("Incorrect changeId '" + changeId + "' or channel " + channelNameToPublish + " for command" + command);
+            }
             return true;
         }
         return false;

diff --git a/src/main/java/commands/ReviewCommandProcessor.java b/src/main/java/commands/ReviewCommandProcessor.java
@@ -19,6 +19,8 @@
 import com.ullink.slack.review.subscription.SubscriptionService;
 import com.ullink.slack.simpleslackapi.SlackSession;
 import com.ullink.slack.simpleslackapi.events.SlackMessagePosted;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Singleton
 public class ReviewCommandProcessor implements SlackBotCommandProcessor
@@ -34,6 +36,8 @@ public class ReviewCommandProcessor implements SlackBotCommandProcessor
     @Inject
     private ChangeInfoFormatter changeInfoDecorator;
 
+    private static final Logger LOGGER       = LoggerFactory.getLogger(ReviewCommandProcessor.class);
+
     private static final String COMMAND = "!review";
     private static Pattern REVIEW_PATTERN = Pattern.compile(COMMAND
         + "((" + SPACES + CHANGE_ID + ")+)"
@@ -45,11 +49,18 @@ public boolean process(String command, SlackMessagePosted event, SlackSession se
         Matcher matcher = REVIEW_PATTERN.matcher(command);
         if (matcher.matches())
         {
-            String[] changeIds = matcher.group(1).split(SPACES);
+            String[] changeIds = matcher.group(1).trim().split(SPACES);
             String comment = matcher.group(4);
             for (int i = 0; i < changeIds.length; i++)
             {
-                executor.execute(new PublishMessageJob(event.getChannel(), changeIds[i].trim(), comment, session, reviewRequestService, subscriptionService, gerritChangeInfoService, changeInfoDecorator));
+                if (changeIds[i] != null && !changeIds[i].trim().isEmpty())
+                {
+                    executor.execute(new PublishMessageJob(event.getChannel(), changeIds[i].trim(), comment, session, reviewRequestService, subscriptionService, gerritChangeInfoService, changeInfoDecorator));
+                }
+                else
+                {
+                    LOGGER.error("Incorrect changeId '" + changeIds[i] + "' for command" + command);
+                }
             }
             return true;
         }

diff --git a/src/main/java/jobs/DeleteMessageJob.java b/src/main/java/jobs/DeleteMessageJob.java
@@ -1,13 +1,17 @@
 package jobs;
 
 import java.util.Collection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import com.ullink.slack.review.gerrit.reviewrequests.ReviewRequest;
 import com.ullink.slack.review.gerrit.reviewrequests.ReviewRequestService;
 import com.ullink.slack.simpleslackapi.SlackChannel;
 import com.ullink.slack.simpleslackapi.SlackSession;
 
 public class DeleteMessageJob implements Runnable
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(DeleteMessageJob.class);
+
     private String                           changeId;
     private final SlackSession               session;
     private final ReviewRequestService       reviewRequestService;
@@ -22,16 +26,26 @@ public DeleteMessageJob(String changeId, SlackSession session, ReviewRequestServ
     @Override
     public void run()
     {
-        Collection<ReviewRequest> reviewRequests = reviewRequestService.getReviewRequests(changeId);
-        for (ReviewRequest reviewRequest : reviewRequests)
+        try
         {
-            SlackChannel channel = session.findChannelById(reviewRequest.getChannelId());
-            if (channel != null)
+            Collection<ReviewRequest> reviewRequests = reviewRequestService.getReviewRequests(changeId);
+            for (ReviewRequest reviewRequest : reviewRequests)
             {
-                session.deleteMessage(reviewRequest.getLastRequestTimestamp(), channel);
+                SlackChannel channel = session.findChannelById(reviewRequest.getChannelId());
+                if (channel != null)
+                {
+                    session.deleteMessage(reviewRequest.getLastRequestTimestamp(), channel);
+                }
+                else
+                {
+                    LOGGER.warn("Cannot find channel " + reviewRequest.getChannelId() + " for delete request " + reviewRequest + " with changeId " + reviewRequest.getChangeId());
+                }
             }
+            reviewRequestService.deleteReviewRequest(changeId);
+        }
+        catch (Throwable t)
+        {
+            LOGGER.error("Unexpected error, t");
         }
-        reviewRequestService.deleteReviewRequest(changeId);
-
     }
 }
diff --git a/src/main/java/jobs/PublishMessageJob.java b/src/main/java/jobs/PublishMessageJob.java
@@ -122,5 +122,9 @@ public void run()
             session.sendMessage(fromChannel, "Could not find change id *`" + changeId + "`*, check that the change id is valid and does not correspond to a draft", null, SlackChatConfiguration.getConfiguration().asUser());
             LOGGER.error("Could not publish review for change id " + changeId, e);
         }
+        catch (Throwable t)
+        {
+            LOGGER.error("Unexpected error, could not publish review for change id `" + changeId + "'", t);
+        }
     }
 }
diff --git a/src/main/java/jobs/RefreshMessageJob.java b/src/main/java/jobs/RefreshMessageJob.java
@@ -2,9 +2,12 @@
 
 import java.io.IOException;
 import java.util.Collection;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import com.ullink.slack.review.gerrit.ChangeInfo;
 import com.ullink.slack.review.gerrit.ChangeInfoFormatter;
 import com.ullink.slack.review.gerrit.GerritChangeInfoService;
+import com.ullink.slack.review.gerrit.ReviewRequestCleanupTask;
 import com.ullink.slack.review.gerrit.reviewrequests.ReviewRequest;
 import com.ullink.slack.review.gerrit.reviewrequests.ReviewRequestService;
 import com.ullink.slack.simpleslackapi.ChannelHistoryModule;
@@ -17,6 +20,8 @@
 
 public class RefreshMessageJob implements Runnable
 {
+    private static final Logger LOGGER = LoggerFactory.getLogger(RefreshMessageJob.class);
+
     private final String                           changeId;
     private final SlackSession               session;
     private final ReviewRequestService       reviewRequestService;
@@ -56,10 +61,16 @@ public void run()
                         }
                         session.updateMessage(reviewRequest.getLastRequestTimestamp(), channel, messageContent, new SlackAttachment[] {attachment});
                     }
+                    else
+                    {
+                        LOGGER.warn("Cannot find channel " + reviewRequest.getChannelId() + " for refresh request " + reviewRequest + " with changeId " + reviewRequest.getChangeId());
+                    }
                 }
             }
-        } catch (IOException e) {
-            e.printStackTrace();
+        }
+        catch (Throwable e)
+        {
+            LOGGER.error("Unexpected error, t");
         }
     }
 }