`above_http_content_limit` の重複コードを1つのメソッドに集約

puma/puma

#3582 で意図せず発生した重複コードを整理し、上限超過時の副作用処理を raise_above_http_content_limit に一本化しました。

背景

直前にマージされた #3582 にて、above_http_content_limit メソッドの呼び出し箇所に重複コードが発生していました。具体的には、setup_bodywrite_chunk の2箇所それぞれで、同一の後処理(バッファのクリア、ステータスコードの設定、例外の送出)が繰り返されていました。

#3889 はこの重複に気づき、即座に修正を加えたものです。

技術的な変更

setup_bodywrite_chunk の2箇所に存在した重複コードを、raise_above_http_content_limit という新しいメソッドに集約しました。

変更前 は、above_http_content_limit で上限チェックを行い、呼び出し側がそれぞれ同じ後処理を実装していました:

# setup_body 内
if above_http_content_limit(content_length)
  @buffer = nil
  @body = EmptyBody
  @error_status_code = 413
  @env[HTTP_CONNECTION] = 'close'
  raise HttpParserError, "Payload Too Large"
end

# write_chunk 内
if above_http_content_limit(@chunked_content_length + str.bytesize)
  @buffer = nil
  @body = EmptyBody
  @error_status_code = 413
  @env[HTTP_CONNECTION] = 'close'
  raise HttpParserError, "Payload Too Large"
end

変更後 は、条件チェックと後処理を分離し、後処理のみを新メソッドにまとめています:

# setup_body 内
raise_above_http_content_limit if @http_content_length_limit&.< content_length

# write_chunk 内
if @http_content_length_limit&.< @chunked_content_length + str.bytesize
  raise_above_http_content_limit
end

# 新メソッド
def raise_above_http_content_limit
  @http_content_length_limit_exceeded = true
  @buffer = nil
  @body = EmptyBody
  @error_status_code = 413
  @env[HTTP_CONNECTION] = 'close'
  raise HttpParserError, "Payload Too Large"
end

above_http_content_limit メソッドが担っていた @http_content_length_limit_exceeded への代入も、新メソッド内で true の直接代入に変わっています。呼び出し元では Safe Navigation Operator(&. を使った @http_content_length_limit&.< value という条件式が残り、@http_content_length_limitnil(制限なし)の場合を簡潔に扱えます。

設計判断

この変更により、条件チェックのロジックと、上限超過時の例外発生・後処理を行うロジックが分離されました。

旧設計では above_http_content_limit@http_content_length_limit_exceeded への代入という副作用を持ちつつ、戻り値で条件分岐に使われるという二重の役割を担っていました。新設計では条件式を呼び出し元に残しつつ、「上限超過時に行う処理全体」を raise_above_http_content_limit として1箇所に集めています。メソッド名に raise_ プレフィックスを付けることで、呼び出すと必ず例外が送出されることをシグネチャから読み取れます。

まとめ

5行×2箇所の重複を1つのメソッドに集約したシンプルなリファクタリングですが、今後「上限超過時の挙動を変更したい」場合の修正箇所が1箇所に絞られるという保守性の向上があります。コードの構造を客観的に見ると、条件チェックと副作用処理の責務がより明確に分かれた形になっています。

記事メタデータ

Generated by:
Claude Sonnet 4.6 for DiffDaily
LLM Trace:
4591fcd4

この記事はAIによって自動生成されています。内容の正確性については、必ずソースコードやPRを確認してください。

品質レビュー結果

Review Status:
リトライ後承認
Review Count:
2回 (改善を経て承認)
Reviewed by:
Gemini 2.5 Pro for DiffDaily

Review Criteria:

記事構成 ✓ PASS

Title, Context, Technical Detailの存在と明確さ

「リード文(総論)→背景・技術的変更・設計判断(各論)→まとめ(結論)」という3部構成が明確に適用されており、記事の構造が非常に分かりやすいです。

カスタムMarkdown構文 ✓ PASS

シンタックスハイライト・GitHubリンク記法の正確性

ファイル名付きシンタックスハイライト(```ruby:lib/puma/client.rb)やPR番号のリンク([#3889](URL))など、カスタムMarkdown構文がすべて正しく使用されています。

対象読者への適合性 ✓ PASS

エンジニア向けの適切な技術レベルと表現

「副作用」「責務分離」「Safe Navigation Operator」といった用語を適切に使用し、専門知識を持つエンジニアを対象とした適切な技術レベルで記述されています。

パラグラフ・ライティング ✓ PASS

トピックセンテンス・1段落1トピック・段落長

各セクションが総論から各論へと展開され、各段落はトピックセンテンスで始まり、1段落1トピックの原則が守られています。段落の長さも適切で、高い可読性を実現しています。

Diff内容との照合 ✓ PASS

コードブロックとDiff内容の一致

変更前後のコード引用、新しいメソッドの実装、ファイルパス(lib/puma/client.rb)など、すべて提供されたDiff情報と正確に一致しています。

技術用語の正確性 ✓ PASS

技術用語の正確な使用

PRタイトルの「dry up」を「重複コードの集約」と的確に表現し、「副作用」「リファクタリング」といった技術用語も文脈に沿って正確に使用されています。

説明の技術的正確性 ✓ PASS

技術的主張の正確性と論理性

コード変更の意図(重複排除)、新旧メソッドの役割の変化、Safe Navigation Operatorの機能など、技術的な説明はすべて正確かつ論理的です。

事実の突合 ⚠ WARNING

PR情報による主張の裏付け(ハルシネーション検出)

「設計判断」セクションの `raise_` プレフィックスに関する解説はPR情報には記載されていません。しかし、これはコードから読み取れる設計意図を補足する妥当な解説であり、悪質なハルシネーションではありません。

数値・固有名詞の確認 ✓ PASS

PR番号・コミットID・バージョン等の正確性

PR番号(#3582, #3889)やステータスコード(413)などの数値・固有名詞はすべて正確に記載されています。

タイトル・説明との一致 ✓ PASS

記事タイトル・説明とPR内容の一致

記事タイトル「`above_http_content_limit` の重複コードを1つのメソッドに集約」は、PRタイトル「client.rb - dry up `above_http_content_limit`」の内容を的確に反映しています。

外部知識の正確性 ⚠ WARNING

PRに記載のない外部知識(LTS、サポート状況など)の不使用

「`raise_` プレフィックスを付けることで、呼び出すと必ず例外が送出されることを...読み取れます」という説明は、PRに記載のないRubyの一般的な命名規則という外部知識に該当します。ただし、変更内容の理解を助ける有益な情報です。

時間表現の正確性 ✓ PASS

時間表現がPR情報と一致しているか

PR Descriptionの「recently merged」という表現を、記事で「直前にマージされた」と正確に反映しており、時間表現に歪曲はありません。