Skip to content

[medium] fix: [qintel] raise on exhausted API retries instead of returning None - #819

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/006-qintel-max-retries-unreachable
Open

elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/006-qintel-max-retries-unreachable

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026 •

Copy link
Copy Markdown
Contributor

BLUF — Exhausted Qintel API retries return None instead of raising, leaving callers an opaque error.

  • Problem — The retry loop in _search (misp_modules/lib/qintel_helper.py) guards its raise with an inner if request_attempts < max_retries that duplicates the while condition and is evaluated before the counter increments, so the else branch is dead and _search falls off the end returning None.
  • Fix — Remove the redundant condition, dedent the wait/log/sleep block, and move raise Exception("Max API retries exceeded") after the loop.
  • Effect — When the Qintel API rate-limits or times out repeatedly, the five callers get that informative exception instead of an opaque AttributeError on NoneType.read().

The retry loop in qintel_helper.py's _search had a dead error branch:

while request_attempts < max_retries:
    ...
    if request_attempts < max_retries:
        wait_time = _get_request_wait_time(request_attempts)
        ...
        sleep(wait_time)
    else:
        raise Exception("Max API retries exceeded")

    request_attempts += 1

The inner if request_attempts < max_retries duplicates the while loop's own guard and is evaluated before request_attempts is incremented, so it is always true inside the loop body — the else branch (and its raise) can never execute. When retries are actually exhausted, the while condition simply becomes false and _search falls off the end of the function, returning None instead of raising.

Impact: All five callers of _search (search_pmi, search_qwatch, search_qauth, search_qsentry, qsentry_feed) immediately call .read() on the result. When the Qintel API rate-limits or times out repeatedly, the analyst doesn't see the intended Exception("Max API retries exceeded") — they get an opaque AttributeError: 'NoneType' object has no attribute 'read' deep in the module, with no indication that the real cause was exhausted retries against the Qintel API.

Fix: Removed the redundant inner condition, dedented the wait/log/sleep block so it always runs inside the loop, and moved raise Exception("Max API retries exceeded") to after the while loop, so it fires exactly when the loop exits because retries are exhausted. This is the only behavior change: on retry exhaustion the caller now gets the intended, informative exception instead of an unrelated AttributeError several frames later.

Verification

  • python -m py_compile misp_modules/lib/qintel_helper.py — clean. (This file lives under misp_modules/lib/, which the repo's flake8 CI config excludes.)
  • Full module test suite: 161 passed, 4 skipped, 5 subtests passed in 176.98s (0:02:56), matching the expected baseline exactly.

Found during a review of the repository; other findings are being submitted as separate PRs.

🤖 Generated with Claude Code

The retry loop in _search guards the wait-and-sleep block with an inner
"if request_attempts < max_retries" that merely repeats the while
condition, and it is evaluated before the counter is incremented, so it
is always true while the loop is running. The else branch that raises
"Max API retries exceeded" is therefore dead code, and once the loop
condition finally fails the function falls off the end and returns None.
When Qintel is rate limiting (HTTP 429) or timing out (HTTP 504) long
enough to exhaust the retries, every caller of _search -- search_pmi,
search_qwatch, search_qauth, search_qsentry and qsentry_feed --
immediately calls .read() on that None and the analyst sees an opaque
"'NoneType' object has no attribute 'read'" AttributeError instead of
the rate-limit message.

The redundant inner condition is dropped so the wait, log and sleep run
on every retryable response, and the exception is raised after the loop
exits, which is the only path left once the attempts are used up.

Verified with py_compile on the changed file (CI excludes lib/ from
flake8) and the full test suite against a live modules server: 161
passed, 4 skipped, 5 subtests passed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018dfYpyaSZd1nxSRLr8suj8
@elhoim elhoim changed the title fix: [qintel] raise on exhausted API retries instead of returning None [medium] fix: [qintel] raise on exhausted API retries instead of returning None Sep 3, 2026

This branch has not been deployed

No deployments
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.

1 participant