From 8219cbc631ae441036f0621b29272f5adc19651e Mon Sep 17 00:00:00 2001 From: Jonathan Cobb Date: Fri, 17 Jan 2020 21:08:26 -0500 Subject: [PATCH] refactor error catcher --- .../cobbzilla/wizard/dao/AbstractCRUDDAO.java | 5 +- .../server/config/ErrorApiConfiguration.java | 34 -------- .../server/listener/ErrbitConfigListener.java | 87 ++++++++++++++----- 3 files changed, 66 insertions(+), 60 deletions(-) diff --git a/wizard-server/src/main/java/org/cobbzilla/wizard/dao/AbstractCRUDDAO.java b/wizard-server/src/main/java/org/cobbzilla/wizard/dao/AbstractCRUDDAO.java index f045c56..b5b4795 100644 --- a/wizard-server/src/main/java/org/cobbzilla/wizard/dao/AbstractCRUDDAO.java +++ b/wizard-server/src/main/java/org/cobbzilla/wizard/dao/AbstractCRUDDAO.java @@ -159,7 +159,7 @@ public abstract class AbstractCRUDDAO dao.getHibernateTemplate().flush(); } catch (RuntimeException e) { if (log.isDebugEnabled()) { - log.error("create("+entity.getClass().getName()+"/"+json(entity)+"): "+e); + log.debug("create("+entity.getClass().getName()+"/"+json(entity)+"): "+e, e); } else { log.error("create: " + e); } @@ -169,7 +169,8 @@ public abstract class AbstractCRUDDAO } protected static boolean isRawMode() { - return AbstractCRUDDAO.rawMode.get() != null && AbstractCRUDDAO.rawMode.get(); + final Boolean raw = AbstractCRUDDAO.rawMode.get(); + return raw != null && raw; } @Override public E createOrUpdate(@Valid E entity) { diff --git a/wizard-server/src/main/java/org/cobbzilla/wizard/server/config/ErrorApiConfiguration.java b/wizard-server/src/main/java/org/cobbzilla/wizard/server/config/ErrorApiConfiguration.java index 9882076..dd7863b 100644 --- a/wizard-server/src/main/java/org/cobbzilla/wizard/server/config/ErrorApiConfiguration.java +++ b/wizard-server/src/main/java/org/cobbzilla/wizard/server/config/ErrorApiConfiguration.java @@ -1,10 +1,7 @@ package org.cobbzilla.wizard.server.config; -import airbrake.AirbrakeNoticeBuilder; -import airbrake.AirbrakeNotifier; import lombok.*; import lombok.extern.slf4j.Slf4j; -import org.apache.commons.collections.buffer.CircularFifoBuffer; import static java.util.concurrent.TimeUnit.SECONDS; import static org.cobbzilla.util.daemon.ZillaRuntime.empty; @@ -23,37 +20,6 @@ public class ErrorApiConfiguration { @Getter @Setter private int bufferSize = 200; @Getter @Setter private long sendInterval = SECONDS.toMillis(5); - @Getter(lazy=true) private final AirbrakeNotifier notifier = initNotifier(); - - private AirbrakeNotifier initNotifier() { return new AirbrakeNotifier(getUrl()); } - public boolean isValid() { return !empty(getUrl()) && !empty(getKey()) && !empty(getEnv()); } - private final CircularFifoBuffer cache = new CircularFifoBuffer(dupCacheSize); - - public void report(Exception e) { - if (inCache(e)) return; - final AirbrakeNoticeBuilder builder = new AirbrakeNoticeBuilder(getKey(), e, getEnv()); - final int responseCode = getNotifier().notify(builder.newNotice()); - if (responseCode != 200) log.warn("report("+e+"): notifier API returned "+responseCode); - } - - public void report(String s) { - if (inCache(s)) return; - final AirbrakeNoticeBuilder builder = new AirbrakeNoticeBuilder(getKey(), s, getEnv()); - final int responseCode = getNotifier().notify(builder.newNotice()); - if (responseCode != 200) log.warn("report("+s+"): notifier API returned "+responseCode); - } - - public boolean inCache(Object o) { - synchronized (cache) { - if (cache.contains(o)) { - log.warn("inCache(" + o + "): already reported recently"); - return true; - } - cache.add(o); - } - return false; - } - } diff --git a/wizard-server/src/main/java/org/cobbzilla/wizard/server/listener/ErrbitConfigListener.java b/wizard-server/src/main/java/org/cobbzilla/wizard/server/listener/ErrbitConfigListener.java index 660b65f..1701e22 100644 --- a/wizard-server/src/main/java/org/cobbzilla/wizard/server/listener/ErrbitConfigListener.java +++ b/wizard-server/src/main/java/org/cobbzilla/wizard/server/listener/ErrbitConfigListener.java @@ -1,5 +1,7 @@ package org.cobbzilla.wizard.server.listener; +import airbrake.AirbrakeNoticeBuilder; +import airbrake.AirbrakeNotifier; import lombok.extern.slf4j.Slf4j; import org.apache.commons.collections.buffer.CircularFifoBuffer; import org.cobbzilla.util.daemon.ErrorApi; @@ -8,13 +10,13 @@ import org.cobbzilla.wizard.server.RestServer; import org.cobbzilla.wizard.server.RestServerBase; import org.cobbzilla.wizard.server.RestServerLifecycleListenerBase; import org.cobbzilla.wizard.server.config.ErrorApiConfiguration; -import org.cobbzilla.wizard.server.config.RestServerConfiguration; import java.util.ArrayList; import java.util.List; import static org.apache.commons.lang3.exception.ExceptionUtils.getStackTrace; -import static org.cobbzilla.util.daemon.ZillaRuntime.shortError; +import static org.cobbzilla.util.daemon.ZillaRuntime.*; +import static org.cobbzilla.util.string.StringUtil.ellipsis; import static org.cobbzilla.util.system.Sleep.sleep; @Slf4j @@ -25,7 +27,7 @@ public class ErrbitConfigListener extends RestServerLifecycleListenerBase { log.warn("onStart: no error API configured, not starting ErrbitApi error sender"); return; } - final ErrbitApi errorApi = new ErrbitApi(server.getConfiguration()); + final ErrbitApi errorApi = new ErrbitApi(server.getConfiguration().getErrorApi()); RestServerBase.setErrorApi(errorApi); ZillaRuntime.setErrorApi(errorApi); errorApi.start(); @@ -37,34 +39,50 @@ public class ErrbitConfigListener extends RestServerLifecycleListenerBase { private static final String SLEEP_MESSAGE = ErrbitApi.class.getName()+": waiting for more errors"; - private final RestServerConfiguration config; - private final CircularFifoBuffer fifo; + private final String key; + private String env; + private long sendInterval; - ErrbitApi(RestServerConfiguration config) { - this.config = config; - this.fifo = new CircularFifoBuffer(config.getErrorApi().getBufferSize()); + private final CircularFifoBuffer dupCache; + private final CircularFifoBuffer fifo; + private AirbrakeNotifier notifier; + + ErrbitApi(ErrorApiConfiguration errorApi) { + if (errorApi != null && errorApi.isValid()) { + this.key = errorApi.getKey(); + this.env = errorApi.getEnv(); + this.sendInterval = errorApi.getSendInterval(); + this.dupCache = new CircularFifoBuffer(errorApi.getDupCacheSize()); + this.fifo = new CircularFifoBuffer(errorApi.getBufferSize()); + this.notifier = new AirbrakeNotifier(errorApi.getUrl()); + } else { + log.warn("ErrbitApi: errorApi is null or invalid, not running"); + this.key = null; + this.fifo = null; + this.dupCache = null; + } } @Override public void report(Exception e) { - if (config.hasErrorApi()) { + if (key != null) { log.error(e.toString()); synchronized (fifo) { fifo.add(e); } } else { - log.warn("report: could not send exception to error reporting API: "+e, e); + if (log.isWarnEnabled()) log.warn("report: could not send exception to error reporting API: "+e, e); } } @Override public void report(String s) { - if (config.hasErrorApi()) { - log.error(s); + if (key != null) { + if (log.isErrorEnabled()) log.error(s); synchronized (fifo) { fifo.add(s); } } else { - log.warn("report: could not send exception to error reporting API: "+s); + if (log.isWarnEnabled()) log.warn("report: could not send exception to error reporting API: "+s); } } @Override public void report(String s, Exception e) { - report(s+"\nException: "+e+"\n"+getStackTrace(e)); + report(s+"\nException: "+shortError(e)+"\n"+getStackTrace(e)); } public void start() { @@ -75,8 +93,6 @@ public class ErrbitConfigListener extends RestServerLifecycleListenerBase { } @Override public void run() { - final ErrorApiConfiguration errorApi = config.getErrorApi(); - final long sendInterval = errorApi.getSendInterval(); while (true) { try { final List reports; @@ -94,27 +110,50 @@ public class ErrbitConfigListener extends RestServerLifecycleListenerBase { } for (Object o : reports) { if (o == null) { - log.error("ErrbitApi.run: NOT reporting null object"); // should never happen + if (log.isErrorEnabled()) log.error("ErrbitApi.run: NOT reporting null object"); // should never happen } else if (o instanceof Exception) { - log.debug("ErrbitApi.run: reporting Exception: "+shortError((Exception) o)); - errorApi.report((Exception) o); + if (log.isDebugEnabled()) log.debug("ErrbitApi.run: reporting Exception: "+shortError((Exception) o)); + sendReport((Exception) o); } else if (o instanceof String) { - log.debug("ErrbitApi.run: reporting String: "+o); - errorApi.report((String) o); + if (log.isDebugEnabled()) log.debug("ErrbitApi.run: reporting String: "+o); + sendReport((String) o); } else { final String val = o.toString(); - log.warn("ErrbitApi.run: reporting object that is neither Exception nor String (" + o.getClass().getName() + ") as String: " + val); - errorApi.report(val); + if (log.isWarnEnabled()) log.warn("ErrbitApi.run: reporting object that is neither Exception nor String (" + o.getClass().getName() + ") as String: " + val); + sendReport(val); } } } catch (Exception e) { - log.error("ErrbitApi.run: unexpected exception: "+shortError(e)); + if (log.isErrorEnabled()) log.error("ErrbitApi.run: unexpected exception: "+shortError(e)); } } } + + public void sendReport(Exception e) { sendReport(fullError(e)); } + + public void sendReport(String s) { + if (inCache(s)) { + if (log.isWarnEnabled()) log.warn("sendReport(" + ellipsis(s, 100) + "): already reported recently, not resending"); + return; + } + final AirbrakeNoticeBuilder builder = new AirbrakeNoticeBuilder(key, s, env); + final int responseCode = notifier.notify(builder.newNotice()); + if (responseCode != 200) { + if (log.isWarnEnabled()) log.warn("sendReport("+ellipsis(s, 100)+"): notifier API returned "+responseCode); + } + } + + public boolean inCache(String s) { + synchronized (dupCache) { + if (dupCache.contains(s)) return true; + dupCache.add(s); + } + return false; + } + } }