yegor256/0pdd

`/ping-github` rescues only `Octokit::NotFound`; `Unauthorized`/`Forbidden`/`TooManyRequests` crash the endpoint and leave notifications stuck

Open

#852 创建于 2026年5月11日

在 GitHub 查看
 (1 评论) (0 反应) (0 负责人)Ruby (37 fork)github user discovery
bughelp wanted

仓库指标

Star
 (142 star)
PR 合并指标
 (PR 指标待抓取)

描述

What happens

In 0pdd.rb, the /ping-github route iterates over GitHub notifications and posts a reply comment to each. The Octokit call is wrapped in a rescue that catches only Octokit::NotFound:

gh.notifications.map do |n|
  begin
    json = gh.issue_comment(repo, comment)
    ...
  rescue Octokit::NotFound => e
    puts "Failed: #{e.message}"
    next
  end
end
gh.mark_notifications_as_read

If gh.issue_comment, gh.notifications, or gh.rate_limit raises Octokit::Unauthorized, Octokit::Forbidden, or Octokit::TooManyRequests, the exception bubbles out of the map block. Crucially, that aborts the iteration before gh.mark_notifications_as_read is reached, so the next ping cycle re-processes the same notifications, hits the same exception, and aborts again. The endpoint becomes self-reinforcingly stuck on the first transient-or-permission error.

There is no outer rescue covering this Sinatra route — JobCommitErrors only wraps the puzzle-processing job, not the ping endpoint — so the unrescued exception is also surfaced as an HTTP 500 to the caller (the GitHub webhook delivery system or a cron poller), where it will be retried indefinitely.

Same defect family as the recently-accepted bugs zerocracy/judges-action#1357#1362: an Octokit call wrapped in a too-narrow rescue.

What should happen

Extend the rescue to cover the full family of recoverable Octokit errors, and additionally consider moving mark_notifications_as_read outside the rescue so it runs even when some individual notifications failed:

begin
  json = gh.issue_comment(repo, comment)
  ...
rescue Octokit::NotFound,
       Octokit::Unauthorized,
       Octokit::Forbidden,
       Octokit::TooManyRequests => e
  puts "Failed for notification #{n.id}: #{e.message}"
  next
end

Also wrap gh.notifications and gh.rate_limit with the same rescue family so a permission or rate-limit error doesn't kill the endpoint. Result: a single bad notification (deleted repo, revoked token, rate limit) no longer stalls the entire ping pipeline forever.

贡献者指南