Skip to content

Fix card server crash when card count reaches max_cards#3068

Closed
Pr030304 wants to merge 1 commit into
Netflix:masterfrom
Pr030304:fix-card-server-max-cards
Closed

Fix card server crash when card count reaches max_cards#3068
Pr030304 wants to merge 1 commit into
Netflix:masterfrom
Pr030304:fix-card-server-max-cards

Conversation

@Pr030304
Copy link
Copy Markdown

PR Type

  • Bug fix
  • New feature
  • Core Runtime change (higher bar -- see CONTRIBUTING.md)
  • Docs / tooling
  • Refactoring

Summary

Fix the card server crash that occurs when a run has at least max_cards cards. The generator now stops cleanly at the configured limit instead of raising StopIteration from inside the generator, which Python converts into a RuntimeError.

Issue

Fixes #2946

Reproduction

Runtime: local

Commands to run:

python -m pytest test/unit/test_card_server.py -v

Where evidence shows up: test output in the local console

Before (error / log snippet)
RuntimeError: generator raised StopIteration

This happens because cards_for_run manually raises StopIteration once the card count reaches max_cards.

After (evidence that fix works)
test/unit/test_card_server.py::test_cards_for_run_stops_cleanly_at_max_cards PASSED
test/unit/test_card_server.py::test_cards_for_run_returns_all_cards_below_limit PASSED

Root Cause

cards_for_run in metaflow/plugins/cards/card_server.py is implemented as a generator. When the number of yielded cards reaches the configured max_cards, it manually raises StopIteration.

In Python 3, raising StopIteration from inside a generator body is not treated as a normal way to end iteration. Per PEP 479, it is converted into RuntimeError: generator raised StopIteration.

That means a run with enough cards to hit the limit can cause the card server to fail while iterating over cards, instead of stopping iteration cleanly.

There is also an off-by-one issue in the original logic: the counter is incremented before the limit check, so the generator attempts to stop as soon as the count reaches max_cards, rather than yielding up to that limit and then stopping.

Why This Fix Is Correct

The fix replaces the manual StopIteration with a normal generator return, which is the correct way to terminate a generator early.

This restores the intended invariant:

  • cards_for_run should yield at most max_cards cards

  • stopping at the limit should not raise an exception

  • runs below the limit should continue to behave unchanged

  • The change is intentionally minimal:

  • it does not alter card ordering

  • it does not change how cards are fetched

  • it only changes how the generator terminates when the limit is reached

Failure Modes Considered

  1. Off-by-one behavior at the limit
    The updated logic ensures the generator can yield exactly max_cards cards before stopping, instead of failing as soon as the count reaches the limit.

  2. Behavior below the limit
    The fix preserves the existing behavior for runs with fewer than max_cards cards. The added regression test verifies that all cards are still returned in that case.

Tests

  • Unit tests added/updated
  • Reproduction script provided (required for Core Runtime)
  • CI passes
  • If tests are impractical: explain why below and provide manual evidence above

Added regression tests in test/unit/test_card_server.py for:

  • stopping cleanly when the number of cards exceeds max_cards
  • returning all cards when the number of cards is below the limit
    Locally ran:
python -m pytest test/unit/test_card_server.py -v

Non-Goals

This PR does not change:

  • card server request handling outside of cards_for_run
  • the configured max_cards behavior itself
  • card ordering or filtering semantics
  • any UI or frontend behavior

AI Tool Usage

  • No AI tools were used in this contribution
  • AI tools were used (describe below)

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Mar 30, 2026

Greptile Summary

This PR fixes a RuntimeError: generator raised StopIteration crash in cards_for_run (PEP 479) and simultaneously corrects an off-by-one error that caused the generator to stop one card short of max_cards.

Changes:

  • metaflow/plugins/cards/card_server.py: Replaces raise StopIteration with return, and moves the limit check before the yield / counter increment so that exactly max_cards cards are returned instead of max_cards - 1.
  • test/unit/test_card_server.py: Adds two regression tests — one verifying the generator stops cleanly at the configured limit and one verifying all cards below the limit are still returned. Includes os.O_NONBLOCK / fcntl shims for portability on Windows.

Confidence Score: 5/5

  • Safe to merge — minimal, targeted fix with clear correctness rationale and regression tests.
  • The change is two lines in the generator body, directly addressing the PEP 479 crash and the off-by-one error. Both cases are covered by new tests. No other behavior is affected.
  • No files require special attention.

Important Files Changed

Filename Overview
metaflow/plugins/cards/card_server.py Fix for PEP 479 violation (StopIteration inside generator → RuntimeError) and an off-by-one error; generator now correctly yields exactly max_cards cards and terminates cleanly with return.
test/unit/test_card_server.py New test file covering both the at-limit (stops cleanly at max_cards=20 with 25 cards available) and below-limit (returns all 3 cards when limit is 20) cases; includes platform shims for Windows compatibility.

Reviews (1): Last reviewed commit: "Fix card server crash when card count re..." | Re-trigger Greptile

@talsperre talsperre closed this Apr 27, 2026
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.

[BUG] Card server crashes with RuntimeError when run has 20+ cards

2 participants