commit 7f339b6be14ef83302092fe71927b9b737d8e337
author Edwin Kempin <ekempin@google.com> Mon Sep 13 17:10:44 2021 +0200
committer Edwin Kempin <ekempin@google.com> Tue Sep 14 15:30:52 2021 +0200
tree 6076dcf00c06e88a31082ec258c24f2d7939360c
parent 0bfae2f06c143f4245b4ad315f3da4cdeca8ffdb

Capture an auto-trace when sending email fails

If sending the email fails due to an exception, retry once with tracing
enabled. This makes it easier to investigate such exceptions.

The current retrying logic is not covering this since at many places we
ignore (catch and log) exceptions that occur during email sending.

Signed-off-by: Edwin Kempin <ekempin@google.com>
Change-Id: Ic58323ff652e750eee46235bcf7f2adfaedc4df6

java/com/google/gerrit/server/mail/send/EmailArguments.java

diff --git a/java/com/google/gerrit/server/mail/send/EmailArguments.java b/java/com/google/gerrit/server/mail/send/EmailArguments.java
index 258c9af..735e34a 100644
--- a/java/com/google/gerrit/server/mail/send/EmailArguments.java
+++ b/java/com/google/gerrit/server/mail/send/EmailArguments.java
@@ -43,6 +43,7 @@
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeQueryBuilder;
 import com.google.gerrit.server.ssh.SshAdvertisedAddresses;
+import com.google.gerrit.server.update.RetryHelper;
 import com.google.gerrit.server.validators.OutgoingEmailValidationListener;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -86,7 +87,6 @@
   final AllProjectsName allProjectsName;
   final List<String> sshAddresses;
   final SitePaths site;
-
   final Provider<ChangeQueryBuilder> queryBuilder;
   final ChangeData.Factory changeDataFactory;
   final Provider<SoySauce> soySauce;
@@ -97,6 +97,7 @@
   final boolean addInstanceNameInSubject;
   final Provider<String> instanceNameProvider;
   final Provider<CurrentUser> currentUserProvider;
+  final RetryHelper retryHelper;
 
   @Inject
   EmailArguments(
@@ -129,7 +130,8 @@
       OutgoingEmailValidator validator,
       @GerritInstanceName Provider<String> instanceNameProvider,
       @GerritServerConfig Config cfg,
-      Provider<CurrentUser> currentUserProvider) {
+      Provider<CurrentUser> currentUserProvider,
+      RetryHelper retryHelper) {
     this.server = server;
     this.projectCache = projectCache;
     this.permissionBackend = permissionBackend;
@@ -158,8 +160,8 @@
     this.accountQueryProvider = accountQueryProvider;
     this.validator = validator;
     this.instanceNameProvider = instanceNameProvider;
-
     this.addInstanceNameInSubject = cfg.getBoolean("sendemail", "addInstanceNameInSubject", false);
     this.currentUserProvider = currentUserProvider;
+    this.retryHelper = retryHelper;
   }
 }

java/com/google/gerrit/server/mail/send/OutgoingEmail.java

diff --git a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
index ddcc0cf..8547336 100644
--- a/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
+++ b/java/com/google/gerrit/server/mail/send/OutgoingEmail.java
@@ -18,6 +18,7 @@
 import static com.google.gerrit.extensions.client.GeneralPreferencesInfo.EmailStrategy.DISABLED;
 import static java.util.Objects.requireNonNull;
 
+import com.google.common.base.Throwables;
 import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.Nullable;
@@ -34,6 +35,7 @@
 import com.google.gerrit.server.account.AccountState;
 import com.google.gerrit.server.change.NotifyResolver;
 import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.update.RetryableAction.ActionType;
 import com.google.gerrit.server.validators.OutgoingEmailValidationListener;
 import com.google.gerrit.server.validators.ValidationException;
 import com.google.template.soy.jbcsrc.api.SoySauce;
@@ -99,6 +101,25 @@
    * @throws EmailException
    */
   public void send() throws EmailException {
+    try {
+      args.retryHelper
+          .action(
+              ActionType.SEND_EMAIL,
+              "sendEmail",
+              () -> {
+                sendImpl();
+                return null;
+              })
+          .retryWithTrace(Exception.class::isInstance)
+          .call();
+    } catch (Exception e) {
+      Throwables.throwIfUnchecked(e);
+      Throwables.throwIfInstanceOf(e, EmailException.class);
+      throw new EmailException("sending email failed", e);
+    }
+  }
+
+  private void sendImpl() throws EmailException {
     if (!args.emailSender.isEnabled()) {
       // Server has explicitly disabled email sending.
       //

java/com/google/gerrit/server/update/RetryableAction.java

diff --git a/java/com/google/gerrit/server/update/RetryableAction.java b/java/com/google/gerrit/server/update/RetryableAction.java
index fbb643e..f79a849 100644
--- a/java/com/google/gerrit/server/update/RetryableAction.java
+++ b/java/com/google/gerrit/server/update/RetryableAction.java
@@ -57,6 +57,7 @@
     PLUGIN_UPDATE,
     REST_READ_REQUEST,
     REST_WRITE_REQUEST,
+    SEND_EMAIL,
   }
 
   @FunctionalInterface