Email throttling config (#49477)

src/metabase/email.clj

@@ -9,9 +9,11 @@
    [metabase.util.malli.schema :as ms]
    [metabase.util.retry :as retry]
    [postal.core :as postal]
-   [postal.support :refer [make-props]])
+   [postal.support :refer [make-props]]
+   [throttle.core :as throttle])
   (:import
-   (javax.mail Session)))
+   (javax.mail Session)
+   (throttle.core Throttler)))
 
 (set! *warn-on-reflection* true)
 
@@ -95,12 +97,58 @@
                   (assert (#{:tls :ssl :none :starttls} (keyword new-value))))
                 (setting/set-value-of-type! :keyword :email-smtp-security new-value)))
 
+(defsetting email-max-recipients-per-second
+  (deferred-tru "The maximum number of recipients, summed across emails, that can be sent per second.
+                Note that the final email sent before reaching the limit is able to exceed it, if it has multiple recipients.")
+  :export?    true
+  :type       :integer
+  :visibility :settings-manager
+  :audit      :getter)
+
+(defn- make-email-throttler
+  [rate-limit]
+  (throttle/make-throttler
+   :email
+   :attempt-ttl-ms     1000
+   :initial-delay-ms   1000
+   :attempts-threshold rate-limit))
+
+(defonce ^:private email-throttler (when-let [rate-limit (email-max-recipients-per-second)]
+                                     (make-email-throttler rate-limit)))
+
+(defn check-email-throttle
+  "Check if the email throttler is enabled and if so, throttle the email sending based on the total number of recipients.
+
+  We will allow multi-recipient emails to broach the limit, as long as the limit has not been reached yet.
+
+  We want two properties:
+    1. All emails eventually get sent.
+    2. Lowering the threshold must never cause more overflow."
+  [email]
+  (when email-throttler
+    (when-let [recipients (not-empty (into #{} (mapcat email) [:to :bcc]))]
+      (let [throttle-threshold (.attempts-threshold ^Throttler email-throttler)
+            check-one!         #(throttle/check email-throttler true)]
+        (check-one!)
+        (try
+          (dotimes [_ (dec (count recipients))]
+            (throttle/check email-throttler true))
+          (catch Exception _e
+            (log/warn "Email throttling is enabled and the number of recipients exceeds the rate limit per second. Skip throttling."
+                      {:email-subject  (:subject email)
+                       :recipients     (count recipients)
+                       :max-recipients throttle-threshold})))))))
+
 ;; ## PUBLIC INTERFACE
 
-(def ^{:arglists '([smtp-credentials email-details])} send-email!
+(defn send-email!
   "Internal function used to send messages. Should take 2 args - a map of SMTP credentials, and a map of email details.
-   Provided so you can swap this out with an \"inbox\" for test purposes."
-  postal/send-message)
+  Provided so you can swap this out with an \"inbox\" for test purposes.
+
+  If email-rate-limit-per-second is set, this function will throttle the email sending based on the total number of recipients."
+  [smtp-credentials email-details]
+  (check-email-throttle email-details)
+  (postal/send-message smtp-credentials email-details))
 
 (defsetting email-configured?
   "Check if email is enabled and that the mandatory settings are configured."

test/metabase/email_test.clj

@@ -12,7 +12,9 @@
    [metabase.util :as u :refer [prog1]]
    [metabase.util.retry :as retry]
    [metabase.util.retry-test :as rt]
-   [postal.message :as message])
+   [postal.core :as postal]
+   [postal.message :as message]
+   [throttle.core :as throttle])
   (:import
    (java.io File)
    (javax.activation MimeType)))
@@ -359,3 +361,81 @@
               (is (re-find
                    #"(?s)Content-Disposition: attachment.+filename=.+this-is-quite-[\-\s?=0-9a-zA-Z]+-characters.csv"
                    (m/mapply email/send-message! params-with-problematic-file))))))))))
+
+(deftest throttle-test
+  (let [send-email (fn [recipients]
+                     (with-redefs [postal/send-message (fn [& args] (last args))]
+                       (email/send-email!
+                        {}
+                        (merge {:from    "awesome@metabase.com"
+                                :subject "101 Reasons to use Metabase"
+                                :body    "101. Metabase will make you a better person"}
+                               recipients))))]
+    (tu/with-temporary-setting-values
+      [email-smtp-host "fake_smtp_host"
+       email-smtp-port 587]
+      (testing "throttle based on the number of recipients"
+        (testing "with 3 separate emails"
+          (with-redefs [email/email-throttler (#'email/make-email-throttler 3)]
+            (testing "ok if there is no recipient"
+              (is (some? (send-email {}))))
+            (is (some? (send-email {:to ["1@metabase.com"]})))
+            (is (some? (send-email {:bcc ["2@metabase.com"]})))
+            (is (some? (send-email {:to ["3@metabase.com"]})))
+            (is (thrown-with-msg?
+                 Exception
+                 #"Too many attempts!.*"
+                 (send-email {:to ["4@metabase.com"]})))
+            (testing "still ok if there is no recipient"
+              (is (some? (send-email {})))))
+
+          (testing "with 1 small then 1 big event"
+            (with-redefs [email/email-throttler (#'email/make-email-throttler 3)]
+              (is (some? (send-email {:to ["1@metabase.com"]})))
+              (is (some? (send-email {:bcc ["2@metabase.com"]
+                                      :to ["3@metabase.com"]})))
+              (is (thrown-with-msg?
+                   Exception
+                   #"Too many attempts!.*"
+                   (send-email {:to ["4@metabase.com"]})))))))
+
+      (testing "if an email has # of recipients greater than the limit"
+        (testing "we skip throttle check if we haven't reached the limit"
+          (with-redefs [email/email-throttler (#'email/make-email-throttler 3)]
+            (is (some? (send-email {:to ["1@metabase.com"]})))
+            ;; this one got through because we haven't reached the limit
+            (is (some? (send-email {:to ["2@metabase.com" "3@metabase.com"]
+                                    :bcc ["4@metabase.com" "5@metabase.com"]})))
+            (testing "senidng another will fail because we maxed-out the limit"
+              (is (thrown-with-msg?
+                   Exception
+                   #"Too many attempts!.*"
+                   (send-email {:to ["6@metabase.com"]}))))))
+
+        (testing "still throttle if we already at limit"
+          (with-redefs [email/email-throttler (#'email/make-email-throttler 3)]
+            ;; mx otu the limit
+            (is (some? (send-email {:to ["1@metabase.com" "2@metabase.com" "3@metabase.com"]})))
+            (testing "but still max-out the limit"
+              (is (thrown-with-msg?
+                   Exception
+                   #"Too many attempts!.*"
+                   (send-email {:to ["4@metabase.com" "5@metabase.com" "6@metabase.com" "7@metabase.com"]})))))))
+
+      (testing "keep retrying will eventually send the email"
+        (with-redefs [email/email-throttler (throttle/make-throttler
+                                             :email
+                                             :attempt-ttl-ms     100
+                                             :initial-delay-ms   100
+                                             :attempts-threshold 3)]
+          (is (some? (send-email {:to ["1@metabase.com" "2@metabase.com" "3@metabase.com"]})))
+          (is (thrown-with-msg?
+               Exception
+               #"Too many attempts!.*"
+               (send-email {:to ["4@metabase.com"]})))
+          (is (some? (u/poll {:thunk       (fn [] (try (send-email {:to ["4@metabase.com"]})
+                                                       (catch Exception _
+                                                         nil)))
+                              :done?       some?
+                              :timeout-ms  200
+                              :interval-ms 10}))))))))

test_resources/serialization_baseline/settings.yaml

 aggregated-query-row-limit: null
+allowed-iframe-hosts: null
 application-colors: null
 application-favicon-url: null
 application-font: null
@@ -13,6 +14,7 @@ custom-formatting: null
 custom-geojson: null
 custom-geojson-enabled: null
 default-maps-enabled: null
+email-max-recipients-per-second: null
 embedding-homepage: null
 enable-embedding: null
 enable-nested-queries: null
@@ -52,4 +54,3 @@ subscription-allowed-domains: null
 synchronous-batch-updates: null
 unaggregated-query-row-limit: null
 update-channel: null
-allowed-iframe-hosts: null