Skip to content

#1671: fix fragile e.name == :get heuristic in qosearch rate limit check#1780

Open
VasilevNStas wants to merge 5 commits into
zerocracy:masterfrom
VasilevNStas:1671-fragile-heuristic
Open

#1671: fix fragile e.name == :get heuristic in qosearch rate limit check#1780
VasilevNStas wants to merge 5 commits into
zerocracy:masterfrom
VasilevNStas:1671-fragile-heuristic

Conversation

@VasilevNStas

@VasilevNStas VasilevNStas commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

Closes #1671

Fix fragile NoMethodError heuristic for detecting :get method availability in Jp.qosearch.

The original code used rescue NoMethodError => e; raise unless e.name == :get — fragile because proxy wrappers (decoor, intercepted) make respond_to?(:get) unreliable (returns true even when the underlying object lacks :get).

Changes:

  • Keep rescue NoMethodError => e; raise unless e.name == :get for the :get fallback path (works correctly with proxy wrappers)
  • Add rescue StandardError for network/WebMock errors so they don't crash the judge
  • When get('/rate_limit') succeeds but resources.search.remaining is missing → left = nil → search is skipped (quota unavailable)
  • When :get is unavailable → fall back to octo.rate_limit.remaining (core rate limit)

@VasilevNStas

Copy link
Copy Markdown
Contributor Author

@yegor256 plz review

Comment thread lib/qos_search.rb Outdated
raise unless e.name == :get
left = octo.rate_limit.remaining
rescue StandardError => e
$loog.info("Can't fetch /rate_limit: #{e.message}")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@VasilevNStas exception swallowing is a sin

@VasilevNStas
VasilevNStas force-pushed the 1671-fragile-heuristic branch from 2d9e905 to af5471a Compare July 13, 2026 20:14
@VasilevNStas

VasilevNStas commented Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

@yegor256
You are right.
Removed the rescue StandardError — exception swallowing is indeed a sin. If rate limit check fails, the exception now propagates.

plz review this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

lib/qos_search.rb fragile e.name == :get heuristic

2 participants