refactor: webhook への送信と再送を Webhook::Client にまとめる

spec/discord/repository_spec.cr

@@ -0,0 +1,47 @@
+require "../spec_helper"
+require "../../src/discord/repository"
+
+# 送信された本文を記録するだけの webhook クライアント。
+# fail_at 番目（0 始まり）の送信で例外を投げ、送信失敗を再現する。
+private class RecordingClient < Webhook::Client
+  getter bodies = [] of String
+
+  def initialize(@fail_at : Int32? = nil)
+    super("discord", "https://example.com/webhook")
+  end
+
+  def post(body : String)
+    raise "send failed" if @fail_at == @bodies.size
+    @bodies << body
+  end
+end
+
+private def messages(count)
+  Array.new(count) { Notify::Message.new(pretext: "pre", title: "t") }
+end
+
+describe Discord::PostRepository do
+  describe "#send_messages" do
+    # embeds は 1 投稿 10 件までなので、12 件なら 2 投稿に分かれる。
+    it "yields the running total after each post" do
+      client = RecordingClient.new
+      sent = [] of Int32
+      Discord::PostRepository.new(client).send_messages(messages(12)) { |count| sent << count }
+
+      client.bodies.size.should eq 2
+      sent.should eq [10, 12]
+    end
+
+    # 送信済みの投稿までしか既読化させないため、失敗した投稿では yield しない。
+    it "stops yielding at the post that fails" do
+      client = RecordingClient.new(fail_at: 1)
+      sent = [] of Int32
+
+      expect_raises(Exception, "send failed") do
+        Discord::PostRepository.new(client).send_messages(messages(12)) { |count| sent << count }
+      end
+
+      sent.should eq [10]
+    end
+  end
+end

spec/slack/repository_spec.cr

@@ -1,97 +1,47 @@
 require "../spec_helper"
 require "../../src/slack/repository"
 
-# HTTP を張らずに、あらかじめ用意した応答を順に返す送信先。
-# 応答を使い切ったあとは最後の応答を返し続ける。
-private class StubPostRepository < Slack::PostRepository
-  getter attempts = 0
+# 送信された本文を記録するだけの webhook クライアント。
+# fail_at 番目（0 始まり）の送信で例外を投げ、送信失敗を再現する。
+private class RecordingClient < Webhook::Client
+  getter bodies = [] of String
 
-  def initialize(@responses : Array(HTTP::Client::Response))
-    super("https://example.com/webhook")
+  def initialize(@fail_at : Int32? = nil)
+    super("slack", "https://example.com/webhook")
   end
 
-  private def post_json(_body : String) : HTTP::Client::Response
-    @attempts += 1
-    @responses[@attempts - 1]? || @responses.last
+  def post(body : String)
+    raise "send failed" if @fail_at == @bodies.size
+    @bodies << body
   end
 end
 
-# Retry-After を 0 にして、待機でテストが遅くならないようにする。
-private def response(status : Int32, retry_after : String? = "0")
-  headers = HTTP::Headers.new
-  headers["Retry-After"] = retry_after if retry_after
-  HTTP::Client::Response.new(status, body: "", headers: headers)
-end
-
-private def messages(count = 1)
+private def messages(count)
   Array.new(count) { Notify::Message.new(pretext: "pre") }
 end
 
 describe Slack::PostRepository do
   describe "#send_messages" do
-    it "sends once and yields the total when the webhook succeeds" do
-      poster = StubPostRepository.new([response(200)])
+    it "sends every message as one post and yields the total" do
+      client = RecordingClient.new
       sent = [] of Int32
-      poster.send_messages(messages(3)) { |count| sent << count }
+      Slack::PostRepository.new(client).send_messages(messages(3)) { |count| sent << count }
 
-      poster.attempts.should eq 1
+      client.bodies.size.should eq 1
+      Slack::Post.from_json(client.bodies.first).attachments.size.should eq 3
       sent.should eq [3]
     end
 
-    # 応答を検査しないと、投稿されていない通知まで呼び出し側が既読化して
-    # 通知が消える。失敗時は yield させず例外にする（issue #120）。
-    it "raises without yielding when the webhook keeps rate limiting" do
-      poster = StubPostRepository.new([response(429)])
-      sent = [] of Int32
-
-      expect_raises(Exception, /slack webhook returned 429/) do
-        poster.send_messages(messages(2)) { |count| sent << count }
-      end
-
-      poster.attempts.should eq Slack::PostRepository::MAX_SEND_ATTEMPTS
-      sent.should be_empty
-    end
-
-    it "raises without yielding when the webhook returns a server error" do
-      poster = StubPostRepository.new([response(500)])
+    # 投稿できていない通知を既読化させないため、失敗時は yield させない。
+    it "does not yield when the post fails" do
+      client = RecordingClient.new(fail_at: 0)
       sent = [] of Int32
 
-      expect_raises(Exception, /slack webhook returned 500/) do
-        poster.send_messages(messages(2)) { |count| sent << count }
+      expect_raises(Exception, "send failed") do
+        Slack::PostRepository.new(client).send_messages(messages(2)) { |count| sent << count }
       end
 
       sent.should be_empty
     end
-
-    it "yields after a retried rate limit succeeds" do
-      poster = StubPostRepository.new([response(429), response(200)])
-      sent = [] of Int32
-      poster.send_messages(messages(2)) { |count| sent << count }
-
-      poster.attempts.should eq 2
-      sent.should eq [2]
-    end
-
-    # 恒久的な失敗を再送しても通らないため、そのまま例外にする。
-    it "does not retry a permanent client error" do
-      poster = StubPostRepository.new([response(404)])
-      sent = [] of Int32
-
-      expect_raises(Exception, /slack webhook returned 404/) do
-        poster.send_messages(messages) { |count| sent << count }
-      end
-
-      poster.attempts.should eq 1
-      sent.should be_empty
-    end
-
-    # Retry-After が無い応答でも待機上限を超えないこと。
-    it "caps the wait when the webhook sends no Retry-After" do
-      poster = StubPostRepository.new([response(429, retry_after: nil), response(200)])
-      started = Time.monotonic
-      poster.send_messages(messages) { }
-
-      (Time.monotonic - started).should be < Slack::PostRepository::MAX_RETRY_WAIT
-    end
   end
 end

spec/webhook/client_spec.cr

@@ -0,0 +1,84 @@
+require "../spec_helper"
+require "../../src/webhook/client"
+
+# HTTP を張らずに、あらかじめ用意した応答を順に返すクライアント。
+# 応答を使い切ったあとは最後の応答を返し続ける。
+private class StubClient < Webhook::Client
+  getter attempts = 0
+
+  def initialize(@responses : Array(HTTP::Client::Response))
+    super("stub", "https://example.com/webhook")
+  end
+
+  private def post_json(_body : String) : HTTP::Client::Response
+    @attempts += 1
+    @responses[@attempts - 1]? || @responses.last
+  end
+end
+
+# Retry-After を 0 にして、待機でテストが遅くならないようにする。
+private def response(status : Int32, retry_after : String? = "0")
+  headers = HTTP::Headers.new
+  headers["Retry-After"] = retry_after if retry_after
+  HTTP::Client::Response.new(status, body: "", headers: headers)
+end
+
+describe Webhook::Client do
+  describe "#post" do
+    it "sends once when the webhook succeeds" do
+      client = StubClient.new([response(200)])
+      client.post "{}"
+
+      client.attempts.should eq 1
+    end
+
+    it "retries a rate limit and returns once it succeeds" do
+      client = StubClient.new([response(429), response(200)])
+      client.post "{}"
+
+      client.attempts.should eq 2
+    end
+
+    it "retries a server error and returns once it succeeds" do
+      client = StubClient.new([response(500), response(200)])
+      client.post "{}"
+
+      client.attempts.should eq 2
+    end
+
+    # 失敗を握りつぶすと、投稿されていない通知まで呼び出し側が既読化する。
+    it "raises after exhausting the attempts on a rate limit" do
+      client = StubClient.new([response(429)])
+
+      expect_raises(Exception, /stub webhook returned 429/) { client.post "{}" }
+
+      client.attempts.should eq Webhook::Client::MAX_SEND_ATTEMPTS
+    end
+
+    it "raises after exhausting the attempts on a server error" do
+      client = StubClient.new([response(500)])
+
+      expect_raises(Exception, /stub webhook returned 500/) { client.post "{}" }
+
+      client.attempts.should eq Webhook::Client::MAX_SEND_ATTEMPTS
+    end
+
+    # 再送しても通らないため、そのまま例外にする。
+    it "does not retry a permanent client error" do
+      client = StubClient.new([response(404)])
+
+      expect_raises(Exception, /stub webhook returned 404/) { client.post "{}" }
+
+      client.attempts.should eq 1
+    end
+
+    # Retry-After が無い応答でも待機上限を超えないこと。
+    it "caps the wait when the webhook sends no Retry-After" do
+      client = StubClient.new([response(429, retry_after: nil), response(200)])
+      started = Time.monotonic
+      client.post "{}"
+
+      (Time.monotonic - started).should be < Webhook::Client::MAX_RETRY_WAIT
+    end
+  end
+end

src/discord/repository.cr

@@ -1,19 +1,17 @@
 require "json"
-require "uri"
-require "http/client"
 require "./models"
 require "../notify/models"
 require "../notify/repository"
 require "../runtime/lambda"
+require "../webhook/client"
 
 module Discord
   class PostRepository < Notify::PostRepository
-    MAX_SEND_ATTEMPTS = 3         # 送信リトライ回数の上限
-    MAX_RETRY_WAIT    = 5.seconds # Retry-After の待機上限
-
     def initialize(url : String)
-      @uri = URI.parse url
-      @client = HTTP::Client.new @uri
+      @webhook = Webhook::Client.new "discord", url
+    end
+
+    def initialize(@webhook : Webhook::Client)
     end
 
     def send_messages(messages : Array(Notify::Message), & : Int32 ->)
@@ -33,31 +31,7 @@ module Discord
       # 送信ペイロードを記録し、content（セリフ）や description / footer が意図通りか
       # CloudWatch で確認できるようにする（issue #95 の切り分け用）。
       Serverless::Lambda.print_log "discord payload: #{body}"
-      headers = HTTP::Headers{"Content-Type" => "application/json"}
-
-      attempt = 0
-      loop do
-        attempt += 1
-        res = @client.post(@uri.request_target, headers: headers, body: body)
-        return if res.success?
-
-        # 通知が複数投稿に分割される場合、前半チャンク送信後に後半が 429/5xx で
-        # 失敗すると既読化されず、次回実行で前半が重複投稿される。これを避けるため
-        # レート制限（429）と一時的な 5xx は Retry-After に従って再送する。
-        retryable = res.status.code == 429 || res.status.server_error?
-        if retryable && attempt < MAX_SEND_ATTEMPTS
-          sleep retry_after(res)
-          next
-        end
-
-        # 恒久的な失敗時は例外にして既読化を止め、次回実行に委ねる。
-        raise "discord webhook returned #{res.status_code}: #{res.body}"
-      end
-    end
-
-    private def retry_after(res : HTTP::Client::Response) : Time::Span
-      seconds = res.headers["Retry-After"]?.try(&.to_f?) || 1.0
-      seconds.seconds.clamp(Time::Span.zero, MAX_RETRY_WAIT)
+      @webhook.post body
     end
   end
 end

src/slack/repository.cr

@@ -1,60 +1,24 @@
 require "json"
-require "uri"
-require "http/client"
 require "./models"
 require "../notify/models"
 require "../notify/repository"
+require "../webhook/client"
 
 module Slack
   class PostRepository < Notify::PostRepository
-    MAX_SEND_ATTEMPTS = 3         # 送信リトライ回数の上限
-    MAX_RETRY_WAIT    = 5.seconds # Retry-After の待機上限
-
     def initialize(url : String)
-      @uri = URI.parse url
-      @client = HTTP::Client.new @uri
+      @webhook = Webhook::Client.new "slack", url
+    end
+
+    def initialize(@webhook : Webhook::Client)
     end
 
     def send_messages(messages : Array(Notify::Message), & : Int32 ->)
       # Slack は全 attachments を 1 投稿で送るため atomic。送信成功後に
       # 全件をまとめて既読化できるよう、累計件数を一度だけ yield する。
-      send_post Post.build(messages)
+      # 送信に失敗すると Webhook::Client が例外にするので、yield には到達しない。
+      @webhook.post Post.build(messages).to_json
       yield messages.size
     end
-
-    private def send_post(post : Post)
-      body = post.to_json
-
-      attempt = 0
-      loop do
-        attempt += 1
-        res = post_json body
-        return if res.success?
-
-        # 重要度で投稿を分けるようになり、1 実行あたりの投稿数が増えてレート制限
-        # （429）に当たりやすくなった（issue #120）。一時的な失敗でその実行を丸ごと
-        # 落とさずに済むよう、429 と 5xx は Retry-After に従って再送する。
-        # 方針は Discord::PostRepository と揃えている。
-        retryable = res.status.code == 429 || res.status.server_error?
-        if retryable && attempt < MAX_SEND_ATTEMPTS
-          sleep retry_after(res)
-          next
-        end
-
-        # 応答を捨てると、投稿されていない通知まで呼び出し側が既読化してしまい、
-        # その通知はどこにも表示されないまま消える。恒久的な失敗は例外にして
-        # 既読化を止め、次回実行に委ねる。
-        raise "slack webhook returned #{res.status_code}: #{res.body}"
-      end
-    end
-
-    private def post_json(body : String) : HTTP::Client::Response
-      @client.post(@uri.request_target, body: body)
-    end
-
-    private def retry_after(res : HTTP::Client::Response) : Time::Span
-      seconds = res.headers["Retry-After"]?.try(&.to_f?) || 1.0
-      seconds.seconds.clamp(Time::Span.zero, MAX_RETRY_WAIT)
-    end
   end
 end