概要
processFeedbackMessage() は report.id による冪等化を持たないため、GitHub Issue 作成に成功したあとで例外が出ると、queue の再試行で TrainLCD/Issues に同じフィードバックの Issue がもう1件作られる。Discord 通知も同様に重複する。
CodeRabbit が #19 のレビューで指摘したもの。#19 は設定変更のみに絞ったため、そちらでは対応していない。
問題のコード
src/consumers/feedbackTriage.ts:1126 にこう書かれている。
// 注意: ここから先(GitHub Issue 作成後)の Discord 通知は失敗しても throw しない。
// throw すると queue ハンドラが retry し、同一レポートで Issue が重複作成されるため、
// 通知の失敗・URL 未設定はログに留める。
意図は正しいが、実装がこの意図を満たしていない。whRes.ok が false のケースは握りつぶしているものの、fetch() 自体が throw するケースが抜けている。
Discord への fetch()(feedbackTriage.ts:1135, :1159)は try(:943)の内側にあり、ネットワークエラーや DNS 失敗、不正な URL では whRes を得る前に throw する。それを :1173 の catch が拾い、:1176 で再送出する。src/index.ts:79 の catch が message.retry() を呼び、再試行では :944 の Issue 作成から丸ごとやり直しになる。
同じ理由で :1015 の const issuesRes = await res.json() も try の内側にあり、ここで throw しても Issue は作成済みのまま再試行される。
再現条件
- フィードバックが queue に入る
TrainLCD/Issues への起票が成功(Issue #N が作られる)
- Discord webhook への
fetch() がネットワークエラーで throw
- 再試行で Issue #N+1 が作られる
max_retries: 3 なので、最悪 4 件の重複 Issue ができる。#19 で DLQ を入れたので、使い切ったメッセージは DLQ に残る。そこから再実行すると、さらにもう1件増える。
なお src/index.ts:76-83 はメッセージ単位で try/catch して retry() するので、バッチ内の他のメッセージが巻き込まれることはない。
影響
- 起票は成功しているのでフィードバックは失われない。純粋に重複起票の問題
- 発生には Discord webhook 側の障害が必要なので頻度は低い
- 公開リポジトリへのスタブ起票(
createPublicIssue())と相互リンクコメント(linkPublicIssue())は内部で catch して null を返す作りなので、この経路では重複しない
対応案
report.id をキーにした永続的な冪等化。STATE_KV に処理済みマーカー(作成した Issue 番号など)を書き、processFeedbackMessage() の入口で既存を確認して、済んでいる工程を飛ばす。
Issue 作成の直後にマーカーを書けば、再試行時に起票を飛ばして Discord 通知から再開できる。KV は結果整合なので厳密な排他にはならないが、この経路(同一メッセージの逐次再試行、間隔は数秒以上)なら実用上は足りるはず。
より単純に、Discord 通知部分を try(:943)の外に出す、あるいは通知全体を個別の try/catch で包んでコメントの意図通り絶対に throw させない、という案もある。こちらは冪等化にはならないが、既知の発生経路は塞げる。
概要
processFeedbackMessage()はreport.idによる冪等化を持たないため、GitHub Issue 作成に成功したあとで例外が出ると、queue の再試行でTrainLCD/Issuesに同じフィードバックの Issue がもう1件作られる。Discord 通知も同様に重複する。CodeRabbit が #19 のレビューで指摘したもの。#19 は設定変更のみに絞ったため、そちらでは対応していない。
問題のコード
src/consumers/feedbackTriage.ts:1126にこう書かれている。意図は正しいが、実装がこの意図を満たしていない。
whRes.okが false のケースは握りつぶしているものの、fetch()自体が throw するケースが抜けている。Discord への
fetch()(feedbackTriage.ts:1135,:1159)はtry(:943)の内側にあり、ネットワークエラーや DNS 失敗、不正な URL ではwhResを得る前に throw する。それを:1173の catch が拾い、:1176で再送出する。src/index.ts:79の catch がmessage.retry()を呼び、再試行では:944の Issue 作成から丸ごとやり直しになる。同じ理由で
:1015のconst issuesRes = await res.json()も try の内側にあり、ここで throw しても Issue は作成済みのまま再試行される。再現条件
TrainLCD/Issuesへの起票が成功(Issue #N が作られる)fetch()がネットワークエラーで throwmax_retries: 3なので、最悪 4 件の重複 Issue ができる。#19 で DLQ を入れたので、使い切ったメッセージは DLQ に残る。そこから再実行すると、さらにもう1件増える。なお
src/index.ts:76-83はメッセージ単位で try/catch してretry()するので、バッチ内の他のメッセージが巻き込まれることはない。影響
createPublicIssue())と相互リンクコメント(linkPublicIssue())は内部で catch して null を返す作りなので、この経路では重複しない対応案
report.idをキーにした永続的な冪等化。STATE_KVに処理済みマーカー(作成した Issue 番号など)を書き、processFeedbackMessage()の入口で既存を確認して、済んでいる工程を飛ばす。Issue 作成の直後にマーカーを書けば、再試行時に起票を飛ばして Discord 通知から再開できる。KV は結果整合なので厳密な排他にはならないが、この経路(同一メッセージの逐次再試行、間隔は数秒以上)なら実用上は足りるはず。
より単純に、Discord 通知部分を
try(:943)の外に出す、あるいは通知全体を個別の try/catch で包んでコメントの意図通り絶対に throw させない、という案もある。こちらは冪等化にはならないが、既知の発生経路は塞げる。