From 24d261875fabc4b5aca8d6c7a44e4129876ae95c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 07:29:08 +0000 Subject: [PATCH 1/2] =?UTF-8?q?refactor:=20webhook=20=E3=81=B8=E3=81=AE?= =?UTF-8?q?=E9=80=81=E4=BF=A1=E3=81=A8=E5=86=8D=E9=80=81=E3=82=92=20Webhoo?= =?UTF-8?q?k::Client=20=E3=81=AB=E3=81=BE=E3=81=A8=E3=82=81=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Slack と Discord のアダプタは投稿の組み立て方こそ違うが、「webhook URL に JSON を POST し、一時的な失敗は再送する」点は同じで、再送の判断と待機が二重に書かれていた （issue #123）。片方だけ直すと挙動がずれるため、送信そのものを切り出す。 まず受け皿となる Webhook::Client を追加する。アダプタの移行は次のコミットで行う。 送信の実体を post_json に分けてあるので、HTTP を張らずに再送の判断を試せる。 Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01SmoDC7XRgDx1AcWumgv3Uq --- spec/webhook/client_spec.cr | 84 +++++++++++++++++++++++++++++++++++++ src/webhook/client.cr | 59 ++++++++++++++++++++++++++ 2 files changed, 143 insertions(+) create mode 100644 spec/webhook/client_spec.cr create mode 100644 src/webhook/client.cr diff --git a/spec/webhook/client_spec.cr b/spec/webhook/client_spec.cr new file mode 100644 index 0000000..b1c8c25 --- /dev/null +++ b/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 diff --git a/src/webhook/client.cr b/src/webhook/client.cr new file mode 100644 index 0000000..92914c3 --- /dev/null +++ b/src/webhook/client.cr @@ -0,0 +1,59 @@ +require "uri" +require "http/client" + +module Webhook + # webhook URL へ JSON を POST する。 + # + # Slack と Discord のアダプタは投稿の組み立て方こそ違うが、「webhook URL に + # JSON を POST し、一時的な失敗は再送する」点は同じで、再送の判断と待機が + # 二重に書かれていた（issue #123）。片方だけ直すと挙動がずれるため、送信そのものは + # ここにまとめ、アダプタは投稿の組み立てに専念する。 + class Client + MAX_SEND_ATTEMPTS = 3 # 送信リトライ回数の上限 + MAX_RETRY_WAIT = 5.seconds # Retry-After の待機上限 + + # service はエラーメッセージに出す送信先の名前。 + def initialize(@service : String, url : String) + @uri = URI.parse url + @client = HTTP::Client.new @uri + end + + # 送信できたら戻り、恒久的に失敗したら例外にする。 + # + # 呼び出し側は投稿の成功を前提に通知を既読化する。応答を見ずに成功扱いにすると、 + # 投稿されていない通知まで既読化され、どこにも表示されないまま消える（issue #120）。 + # よって失敗は握りつぶさず例外にし、既読化を止めて次回実行に委ねる。 + def post(body : String) + attempt = 0 + loop do + attempt += 1 + res = post_json body + return if res.success? + + # 通知は複数の投稿に分割されうるため、1 実行で webhook を連続して叩く。 + # レート制限（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 "#{@service} webhook returned #{res.status_code}: #{res.body}" + end + end + + # 送信の実体。ここだけを差し替えれば、HTTP を張らずに再送の判断を試せる。 + private def post_json(body : String) : HTTP::Client::Response + headers = HTTP::Headers{"Content-Type" => "application/json"} + @client.post(@uri.request_target, headers: headers, body: body) + end + + # 待機時間は Retry-After に従う。ヘッダが無ければ 1 秒、長すぎる指定は + # Lambda の実行時間を食い潰さないよう上限で丸める。 + 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 From dfae35c0903a0696df1a7286775601300647d95c Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 29 Aug 2026 07:29:09 +0000 Subject: [PATCH 2/2] =?UTF-8?q?refactor:=20Slack=20=E3=81=A8=20Discord=20?= =?UTF-8?q?=E3=82=92=20Webhook::Client=20=E3=81=AB=E5=AF=84=E3=81=9B?= =?UTF-8?q?=E3=82=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 両アダプタから再送ループ、Retry-After の解釈、リトライ回数と待機上限の定数を 取り除き、Webhook::Client に委ねる（issue #123）。アダプタに残るのは投稿の 組み立てと、Discord のペイロード記録（issue #95）だけになる。 エラーメッセージの送信先名はクライアントに渡す service で保つため、 `slack webhook returned ...` / `discord webhook returned ...` は変わらない。 Slack はこれまで Content-Type を付けずに送っていたが、共通化にあわせて Discord と 同じく application/json を付ける。Slack の Incoming Webhook が案内している送り方に 揃える形になる。 spec ではクライアントを差し替えられるよう、Webhook::Client を受け取る初期化を 足した。アダプタ側の spec は投稿の分かれ方と yield の有無だけを見る。 Co-Authored-By: Claude Claude-Session: https://claude.ai/code/session_01SmoDC7XRgDx1AcWumgv3Uq --- spec/discord/repository_spec.cr | 47 +++++++++++++++++ spec/slack/repository_spec.cr | 90 ++++++++------------------------- src/discord/repository.cr | 38 +++----------- src/slack/repository.cr | 50 +++--------------- 4 files changed, 80 insertions(+), 145 deletions(-) create mode 100644 spec/discord/repository_spec.cr diff --git a/spec/discord/repository_spec.cr b/spec/discord/repository_spec.cr new file mode 100644 index 0000000..265ddef --- /dev/null +++ b/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 diff --git a/spec/slack/repository_spec.cr b/spec/slack/repository_spec.cr index ad8eb43..78bf3a1 100644 --- a/spec/slack/repository_spec.cr +++ b/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 diff --git a/src/discord/repository.cr b/src/discord/repository.cr index faa639e..ab306f4 100644 --- a/src/discord/repository.cr +++ b/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 diff --git a/src/slack/repository.cr b/src/slack/repository.cr index 841adbd..408dd98 100644 --- a/src/slack/repository.cr +++ b/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