Catch explicit exceptions instead of Exception
It's better to only catch the specific execptions that are actually
thrown from the methods being called.
Also make sure we consistently log errors with the log level error
rather than warn or info, and improve some of the message texts.
Change-Id: I1889cd4da702ce43a4e087f21c27680900abd5a9
diff --git a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/AMQProperties.java b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/AMQProperties.java
index bdc3a88..f38a9cc 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/AMQProperties.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/AMQProperties.java
@@ -63,8 +63,9 @@
default:
break;
}
- } catch (Exception ex) {
- LOGGER.info(ex.getMessage());
+ } catch (IllegalAccessException ex) {
+ LOGGER.warn("Cannot access field {}. Cause: {}",
+ f.getName(), ex.getMessage());
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/section/Sections.java b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/section/Sections.java
index b2361ac..759ef03 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/section/Sections.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/config/section/Sections.java
@@ -48,8 +48,9 @@
f.set(section, Boolean.valueOf(a.value()));
}
}
- } catch (Exception ex) {
- LOGGER.warn("Exception during initialize: {}", f.getName());
+ } catch (IllegalAccessException ex) {
+ LOGGER.warn("Cannot access field {}. Cause: {}",
+ f.getName(), ex.getMessage());
}
}
return section;
@@ -76,9 +77,9 @@
config.setBoolean(getName(section), null, f.getName(), Boolean.class.cast(obj));
}
}
- } catch (Exception ex) {
- LOGGER.warn("Exception during toConfig: {}", f.getName());
- LOGGER.info("{}", ex.getMessage());
+ } catch (IllegalAccessException ex) {
+ LOGGER.warn("Cannot access field {}. Cause: {}",
+ f.getName(), ex.getMessage());
}
}
return config;
@@ -104,8 +105,9 @@
f.set(section, config.getBoolean(getName(section), null, f.getName(), false));
}
}
- } catch (Exception ex) {
- LOGGER.warn("Exception during fromConfig: {}", f.getName());
+ } catch (IllegalAccessException ex) {
+ LOGGER.warn("Cannot access field {}. Cause: {}",
+ f.getName(), ex.getMessage());
}
}
}
@@ -131,9 +133,9 @@
f.set(section, val);
}
}
- } catch (Exception ex) {
- LOGGER.warn("Exception during normalize: {}", f.getName());
- LOGGER.info("{}", ex.getMessage());
+ } catch (IllegalAccessException ex) {
+ LOGGER.warn("Cannot access field {}. Cause: {}",
+ f.getName(), ex.getMessage());
}
}
return section;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/session/type/AMQPSession.java b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/session/type/AMQPSession.java
index 0fa964b..1f9b7e3 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/session/type/AMQPSession.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/session/type/AMQPSession.java
@@ -34,6 +34,9 @@
import java.io.IOException;
import java.net.URISyntaxException;
+import java.security.KeyManagementException;
+import java.security.NoSuchAlgorithmException;
+import java.util.concurrent.TimeoutException;
import java.util.concurrent.atomic.AtomicInteger;
public final class AMQPSession implements Session {
@@ -118,8 +121,8 @@
ch.addShutdownListener(channelListener);
failureCount.set(0);
LOGGER.info(MSG("Channel #{} opened."), ch.getChannelNumber());
- } catch (Exception ex) {
- LOGGER.warn(MSG("Failed to open channel."));
+ } catch (IOException ex) {
+ LOGGER.error(MSG("Failed to open channel."), ex);
failureCount.incrementAndGet();
}
if (failureCount.get() > properties.getSection(Monitor.class).failureCount) {
@@ -155,9 +158,9 @@
} catch (URISyntaxException ex) {
LOGGER.error(MSG("URI syntax error: {}"), amqp.uri);
} catch (IOException ex) {
- LOGGER.error(MSG("Connection cannot be opened."));
- } catch (Exception ex) {
- LOGGER.warn(MSG("Connection has something error. it will be disposed."), ex);
+ LOGGER.error(MSG("Connection cannot be opened."), ex);
+ } catch (KeyManagementException | NoSuchAlgorithmException ex) {
+ LOGGER.error(MSG("Security error when opening connection."), ex);
}
}
@@ -166,22 +169,22 @@
LOGGER.info(MSG("Disconnecting..."));
try {
if (channel != null) {
- LOGGER.info(MSG("Close Channel #{}..."), channel.getChannelNumber());
+ LOGGER.info(MSG("Closing Channel #{}..."), channel.getChannelNumber());
channel.close();
}
- } catch (Exception ex) {
- LOGGER.warn(MSG("Error when close channel.") , ex);
+ } catch (IOException | TimeoutException ex) {
+ LOGGER.error(MSG("Error when closing channel."), ex);
} finally {
channel = null;
}
try {
if (connection != null) {
- LOGGER.info(MSG("Close Connection..."));
+ LOGGER.info(MSG("Closing Connection..."));
connection.close();
}
- } catch (Exception ex) {
- LOGGER.warn(MSG("Error when close connection.") , ex);
+ } catch (IOException ex) {
+ LOGGER.error(MSG("Error when closing connection."), ex);
} finally {
connection = null;
}
@@ -196,12 +199,12 @@
Message message = properties.getSection(Message.class);
Exchange exchange = properties.getSection(Exchange.class);
try {
- LOGGER.debug(MSG("Send message."));
+ LOGGER.debug(MSG("Sending message."));
channel.basicPublish(exchange.name, message.routingKey,
properties.getAMQProperties().getBasicProperties(),
messageBody.getBytes(CharEncoding.UTF_8));
- } catch (Exception ex) {
- LOGGER.warn(MSG("Error when sending meessage."), ex);
+ } catch (IOException ex) {
+ LOGGER.error(MSG("Error when sending meessage."), ex);
}
}
}
diff --git a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/solver/version/V1.java b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/solver/version/V1.java
index 4a9cb9f..800f01b 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/solver/version/V1.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/rabbitmq/solver/version/V1.java
@@ -26,6 +26,7 @@
import org.slf4j.LoggerFactory;
import java.io.File;
+import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -65,8 +66,8 @@
Files.createDirectories(siteDir);
Files.move(oldFile, newFile);
Files.createFile(siteDir.resolve(DEFAULT_SITE_NAME + FILE_EXT));
- } catch (Exception ex) {
- LOGGER.info(ex.getMessage());
+ } catch (IOException ex) {
+ LOGGER.error("Failed to initialize plugin configuration", ex);
}
}
}
