Skip to content

fix: isolate per-miner exceptions in OSS scoring loop - #996

Closed
plind-junior wants to merge 7 commits into
entrius:testfrom
plind-junior:fix/per-miner-exception-isolation
Closed

fix: isolate per-miner exceptions in OSS scoring loop#996
plind-junior wants to merge 7 commits into
entrius:testfrom
plind-junior:fix/per-miner-exception-isolation

Conversation

@plind-junior

Copy link
Copy Markdown
Contributor

Summary

Wrap the per-miner evaluate_miners_pull_requests call in gittensor/validator/oss_contributions/reward.py in try/except Exception so a single miner's uncaught exception cannot abort the whole scoring round.

Without this, any unhandled exception (e.g. RecursionError from #978, ValueError from #893, future schema-drift KeyError / AttributeError) propagates through oss_contributionsforwardconcurrent_forward → the while True at neurons/base/validator.py:144-169. The except Exception there logs once and run() returns. Because run() is the target of a daemon thread (base/validator.py:179), the scoring thread dies silently while the main thread keeps printing Validator running | uid X | ... every 30s — no further forward passes, no weight updates, until manual restart. After restart the validator re-fetches the same data and re-crashes on the same miner.

The crashed miner now gets a MinerEvaluation(uid=uid, hotkey=hotkey, failed_reason=...), so downstream skip-on-failure paths (MinerEvaluationCache.store_or_use_cached_evaluation, mirror_scan, repository.upsert_miner_evaluation) already treat the miner as non-scoring this round without any new per-site awareness.

This is a safety net, not a substitute for fixing each specific exception class at source. It brings the OSS path to parity with issue_discovery/mirror_scan.py:157-162, which already isolates per-miner failures.

Related Issues

Closes #995

Related (independent vectors that fire this chain):

Type of Change

  • Bug fix (defense-in-depth)

Testing

  • Tests added
  • Manually verified loop continues past a forced exception

New tests/validator/oss_contributions/test_reward.py:

  • test_per_miner_exception_does_not_abort_round — boom on UID 3 (raises RecursionError); asserts UIDs 1/2/4/5 are still scored, UID 3 carries failed_reason, get_rewards returns a normal (rewards, evaluations, cached_uids, penalized_uids) tuple.
  • test_failed_reason_records_exception_class_and_message — asserts the substituted MinerEvaluation.failed_reason includes both the exception class name and message, so post-hoc DB inspection can identify the vector that crashed a miner.

Local verification:

pytest tests/validator/oss_contributions/  # 124 passed
ruff check gittensor/validator/oss_contributions/reward.py tests/validator/oss_contributions/test_reward.py  # clean
ruff format --check gittensor/validator/oss_contributions/reward.py tests/validator/oss_contributions/test_reward.py  # clean

Checklist

  • Code follows project style guidelines
  • Self-review completed
  • Tests added
  • ruff check / ruff format --check clean
  • Catches Exception (not BaseException) so KeyboardInterrupt / SystemExit still propagate
  • No behavior change on the happy path — the wrap only adds a fallback

Wrap the per-miner evaluate_miners_pull_requests call in try/except so a
single miner's uncaught exception cannot abort the whole scoring round.
Without this, any RecursionError / ValueError / etc. propagated through
oss_contributions -> forward -> concurrent_forward -> the while-True at
neurons/base/validator.py and silently killed the validator's daemon
scoring thread, leaving the main thread to log "Validator running"
indefinitely with no further forward passes.

The crashed miner gets a MinerEvaluation with failed_reason set, so
downstream skip-on-failure paths (cache, issue discovery, DB write)
treat the miner as non-scoring this round without per-site awareness.

Brings the OSS path to parity with issue_discovery/mirror_scan, which
already isolates per-miner failures.
@xiao-xiao-mao xiao-xiao-mao Bot added the bug Something isn't working label May 5, 2026
The repo doesn't ship pytest-asyncio in its dev extras, so @pytest.mark.asyncio
silently warned-then-failed on CI. Switch to plain sync test functions that
call asyncio.run(get_rewards(...)) — matches the rest of the test suite (no
other test in this repo uses pytest-asyncio).
@anderdc

anderdc commented May 9, 2026

Copy link
Copy Markdown
Collaborator

neurons/validator.py:200-206 already detects scoring-thread death via thread.is_alive() and exits the process within 30s for supervisor restart — the "silent freeze until manual restart" premise doesn't hold. The OSS path is asymmetric with mirror_scan only because mirror_scan catches a specific MirrorRequestError, not bare Exception; this PR is strictly broader and would mask deterministic crashes (#978, #893) that should be fixed at source. Closing.

@anderdc anderdc closed this May 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] reward.py get_rewards loop: any per-miner exception silently kills validator scoring thread

2 participants