Skip to content

[bug] Pool-mode batch-failure paths call non-existent storage.update_batch() → AttributeError (batch_scheduler.py:910,977) #299

Description

@drunkcoding

Summary

Both pool-mode batch-failure paths call self.storage.update_batch(...), but StorageManager has no such method — it only defines update_batch_status(...). When either path fires, the scheduler task raises AttributeError instead of marking the batch failed, so the batch is never moved to a terminal status and the real error is masked.

Verified on origin/main @ d171a15.

The defect

batchgen/server/batch_scheduler.py (pool mode, self._pool_mode):

  • L910 — IntakePool rejection when the pool is at capacity (capacity_exceeded):
    self.storage.update_batch(batch_id, status="failed", error={
        "code": "capacity_exceeded", "message": error_msg,
    })
  • L977 — pool-mode batch timeout / tracker failure (batch_failed):
    self.storage.update_batch(batch_id, status="failed", error={
        "code": "batch_failed", "message": str(error_msg)
    })

Three things are wrong in each call:

  1. Method does not exist. storage.py:103 only defines update_batch_status(self, batch_id, status: BatchStatus, **updates). There is no update_batchAttributeError.
  2. Wrong status type. status="failed" is a str; update_batch_status does data.update({"status": status.value, ...}), i.e. it expects a BatchStatus enum. Passing a str would AttributeError on .value even if the method name were right. Correct value: BatchStatus.FAILED.
  3. Wrong error type. BatchObject.error is Optional[str] (io_struct.py:191), but a dict is passed. Every other caller passes a plain string, e.g. batch_scheduler.py:221:
    self.storage.update_batch_status(batch_id, BatchStatus.FAILED, error=error_message)

Reachability / impact

  • L910 fires under load once the intake pool hits max_capacity (documented capacity_exceeded rejection).
  • L977 fires on batch timeout (self._batch_timeout, default 24h) or a fatal tracker error.
  • Effect: the failure handler itself throws AttributeError; the batch is left non-terminal (stuck), and clients polling GET /v1/batches/{id} never see failed. Legacy (non-pool) mode is unaffected — it already uses update_batch_status correctly.

Proposed fix

Use the established pattern (mirrors batch_scheduler.py:221):

self.storage.update_batch_status(
    batch_id, BatchStatus.FAILED,
    error=error_msg,
)

and analogously at L977 with the batch_failed message. error flows through **updates into the persisted batch JSON, matching the other 8 update_batch_status call sites.

If a structured {code, message} error is actually desired on the batch object, that's a separate enhancement: change BatchObject.error from Optional[str] to the existing Optional[BatchError] model (io_struct.py:246,324) and update all callers consistently. The minimal correctness fix is the string form above.

Suggested regression test

Add a tests/ unit test with a fake/in-memory storage that exercises both pool-mode failure paths (capacity rejection + batch timeout) and asserts the batch reaches status == "failed" with the expected error text — this would have caught the AttributeError.

Provenance

Found during a full fresh re-verification of the v1.0.10.post4 release docs (static cross-check of docs/batch-api*.md / docs/async-batch-submission.md against the server). This is a code bug, not a docs bug, and is independent of the doc mismatches already tracked in #289 / #279.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions