Skip to content

Change future checks from .cancelled() to .done() before setting future results.#55

Merged
markedwards merged 3 commits into
syrusakbary:masterfrom
FerasAlazzeh:fix_invalid_state_error
Nov 28, 2025
Merged

Change future checks from .cancelled() to .done() before setting future results.#55
markedwards merged 3 commits into
syrusakbary:masterfrom
FerasAlazzeh:fix_invalid_state_error

Conversation

@FerasAlazzeh

Copy link
Copy Markdown

Fix: Use .done() instead of .cancelled() to prevent InvalidStateError

Problem

The code was checking if not ql.future.cancelled() before setting results or exceptions on futures. However, a future can be in a "done" state for multiple reasons:

  • Cancelled
  • Has a result set
  • Has an exception set

see: https://docs.python.org/3/library/asyncio-task.html#asyncio.Task.done

If a future already has a result or exception set (but is not cancelled), the cancelled() check would pass, and attempting to call set_result() or set_exception() on an already-done future would raise InvalidStateError.

Solution

Changed all checks from cancelled() to done() in three locations:

  1. dispatch_queue_batch() - when setting results/exceptions from batch load
  2. failed_dispatch() - when setting exceptions after batch failure
  3. prime() - when priming the cache with values

The done() check is more comprehensive as it catches all completion states, preventing InvalidStateError in all scenarios.

Testing

All existing tests pass, and new tests verify that:

  • Futures that are already done (with result or exception) are not overwritten
  • No InvalidStateError exceptions are raised
  • The behavior is correct in both success and failure scenarios

@coveralls

coveralls commented Nov 20, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 92.5%. remained the same
when pulling 47f5937 on FerasAlazzeh:fix_invalid_state_error
into ffad8a1 on syrusakbary:master.

@FerasAlazzeh FerasAlazzeh changed the title Change future checks from .cancelled() to .done() for futures before setting future results. Change future checks from .cancelled() to .done() before setting future results. Nov 21, 2025
@markedwards markedwards merged commit d6319be into syrusakbary:master Nov 28, 2025
8 checks passed
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.

3 participants