Skip to content

fix(batch): set has_errors on per-object/reference errors in stream recv - #2115

Open
vaibzde wants to merge 1 commit into
weaviate:mainfrom
vaibzde:fix/batch-stream-has-errors-flag
Open

fix(batch): set has_errors on per-object/reference errors in stream recv#2115
vaibzde wants to merge 1 commit into
weaviate:mainfrom
vaibzde:fix/batch-stream-has-errors-flag

Conversation

@vaibzde

@vaibzde vaibzde commented Jul 31, 2026

Copy link
Copy Markdown

Closes #2107.

What's broken

_BatchBaseSync.__recv (and the identical code in _BatchBaseAsync) builds a fresh BatchObjectReturn for each failed object coming back on the batch stream:

result_objs += BatchObjectReturn(
    _all_responses=[err],
    errors={cached.index: err},
)

has_errors isn't passed, so it keeps its dataclass default of False. BatchObjectReturn.__add__ only ORs it forward (self.has_errors = self.has_errors or other.has_errors), so it never flips to True even though errors has an entry. Same thing for the reference-error branch (BatchReferenceReturn).

This is the code both data.ingest() and collection.batch.stream() use, so result.has_errors stays False on both, and the idiomatic if result.has_errors: check never fires. failed_objects/failed_references are still correct because those lists are populated separately in the same block, from the raw ErrorObject/ErrorReference, not through the buggy has_errors accumulation — that's why #2107's own repro saw failed_objects come back right on batch.stream() while only checking has_errors on ingest().

Fix

Pass has_errors=True at the four call sites that construct these objects from a server-reported error (2 in sync.py, 2 in async_.py).

How to verify

Added test_ingest_has_errors_on_failed_object to mock_tests/test_batch.py — a mock gRPC BatchStream service that returns one object error, driven through collection.data.ingest(), asserting result.has_errors is True. It fails on main (has_errors comes back False even though errors has one entry) and passes with this fix.

Ran locally (uv venv --python 3.12 .venv, uv pip install -e . -r requirements-test.txt -r requirements-devel.txt):

  • pytest mock_tests/ — 52 passed (including the new test)
  • pytest test/ — 386 passed, 1 skipped
  • ruff format --check / ruff check — clean
  • flake8 --config=.flake8 — clean
  • pyright (v1.1.400, matching .pre-commit-config.yaml) — 0 errors

I didn't run the integration suite (integration/) since it needs a live Weaviate server — deferring to CI there.

What I didn't touch

Left the actual server-side error paths, retry/backoff logic, and failed_objects/failed_references tracking alone — this is scoped to the one missing kwarg per call site.

In _BatchBaseSync/_BatchBaseAsync.__recv, each failed object or reference
from the server was wrapped in a fresh BatchObjectReturn/BatchReferenceReturn
with only `errors` set. has_errors defaults to False on the dataclass and
__add__ only ORs it forward (`self.has_errors or other.has_errors`), so it
never became True even though `errors` was non-empty. This meant
`if result.has_errors:` never fired on data.ingest() (and on batch.stream(),
though failed_objects/failed_references were populated correctly there since
those are tracked separately).

Fixes weaviate#2107

@orca-security-eu orca-security-eu Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Orca Security Scan Summary

Status Check Issues by priority
Passed Passed Infrastructure as Code high 0   medium 0   low 0   info 0 View in Orca
Passed Passed SAST high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Secrets high 0   medium 0   low 0   info 0 View in Orca
Passed Passed Vulnerabilities high 0   medium 0   low 0   info 0 View in Orca

@weaviate-git-bot

Copy link
Copy Markdown

To avoid any confusion in the future about your contribution to Weaviate, we work with a Contributor License Agreement. If you agree, you can simply add a comment to this PR that you agree with the CLA so that we can merge.

beep boop - the Weaviate bot 👋🤖

PS:
Are you already a member of the Weaviate Forum?

@vaibzde

vaibzde commented Jul 31, 2026

Copy link
Copy Markdown
Author

I agree with the CLA.

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.

data.ingest(): BatchObjectReturn.has_errors stays False when errors is populated (and batch.failed_objects is empty)

3 participants