From cfe6ee0afb073ff828b97e63e515a8475a3cb8a7 Mon Sep 17 00:00:00 2001 From: Eric Del Orefice <114540347+ericdelorefice@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:18:58 -0400 Subject: [PATCH 1/3] Python: chroma collection_exists catches too much, ensure_collection_deleted too little pyproject allows chromadb >= 0.5, < 1.6, and the error for a missing collection is not the same across that range. 0.5 has no NotFoundError at all and raised ValueError; 1.5.9 raises chromadb.errors.NotFoundError, which is not a ValueError. Both symptoms follow from that one fact. collection_exists catches bare Exception, so an unreachable server or a rejected request reports that the collection is not there. Three places already say that is not the rule: _get_collection nine lines above raises RuntimeError from the identical client.get_collection call, ensure_collection_deleted raises VectorStoreOperationException for anything that is not a missing collection, and VectorStore.collection_exists in data/vector.py narrows to except VectorStoreOperationException when it wraps this method. ensure_collection_deleted catches only ValueError, the 0.5-era signal, so on chromadb 1.x a genuinely absent collection is re-raised as a VectorStoreOperationException and the "doesn't exist" path is unreachable. One module-level tuple resolves the version difference, behind a guarded import so chromadb 0.5 keeps working. collection_exists narrows to it and ensure_collection_deleted widens to it. Four tests, no server. They resolve the not-found error themselves rather than importing the new constant, so the file runs against the code before and after. Two fail on main, one per symptom, and two pass there. --- python/semantic_kernel/connectors/chroma.py | 18 +++++-- .../unit/connectors/memory/test_chroma.py | 51 +++++++++++++++++++ 2 files changed, 66 insertions(+), 3 deletions(-) diff --git a/python/semantic_kernel/connectors/chroma.py b/python/semantic_kernel/connectors/chroma.py index 2bffb64ecd5e..3b5dac4b5b06 100644 --- a/python/semantic_kernel/connectors/chroma.py +++ b/python/semantic_kernel/connectors/chroma.py @@ -12,6 +12,13 @@ from chromadb.api.types import EmbeddingFunction, Space from chromadb.config import Settings +try: # chromadb >= 1.0 raises NotFoundError for a missing collection + from chromadb.errors import NotFoundError as _ChromaNotFoundError + + COLLECTION_NOT_FOUND: tuple[type[Exception], ...] = (ValueError, _ChromaNotFoundError) +except ImportError: # chromadb < 1.0 raised ValueError + COLLECTION_NOT_FOUND = (ValueError,) + from semantic_kernel.connectors.ai.embedding_generator_base import EmbeddingGeneratorBase from semantic_kernel.data.vector import ( DistanceFunction, @@ -128,11 +135,16 @@ def _get_collection(self) -> Collection: @override async def collection_exists(self, **kwargs: Any) -> bool: - """Check if the collection exists.""" + """Check if the collection exists. + + Returns False only when Chroma says the collection is not there. Anything else - + an unreachable server, a rejected request - is raised, because the caller cannot + tell it apart from an absent collection otherwise. + """ try: self.client.get_collection(name=self.collection_name, embedding_function=self.embedding_func) return True - except Exception: + except COLLECTION_NOT_FOUND: return False @override @@ -184,7 +196,7 @@ async def ensure_collection_deleted(self, **kwargs: Any) -> None: """Delete the collection.""" try: self.client.delete_collection(name=self.collection_name) - except ValueError: + except COLLECTION_NOT_FOUND: logger.info(f"Collection {self.collection_name} could not be deleted because it doesn't exist.") except Exception as e: raise VectorStoreOperationException( diff --git a/python/tests/unit/connectors/memory/test_chroma.py b/python/tests/unit/connectors/memory/test_chroma.py index 6f48fa6663af..e257eea4cff0 100644 --- a/python/tests/unit/connectors/memory/test_chroma.py +++ b/python/tests/unit/connectors/memory/test_chroma.py @@ -7,6 +7,20 @@ from chromadb.api.models.Collection import Collection from semantic_kernel.connectors.chroma import ChromaCollection, ChromaStore +from semantic_kernel.exceptions.vector_store_exceptions import VectorStoreOperationException + + +def _not_found() -> Exception: + """The error Chroma raises for a missing collection, on whichever version is installed. + + Resolved here rather than imported from the connector, so these tests run unchanged + against the code before and after the fix. + """ + try: + from chromadb.errors import NotFoundError # chromadb >= 1.0 + except ImportError: # chromadb < 1.0 raised ValueError + return ValueError("Collection test_collection does not exist.") + return NotFoundError("Collection test_collection does not exist.") @pytest.fixture @@ -113,3 +127,40 @@ async def test_chroma_collection_search(chroma_collection, mock_client, include_ async for res in results.results: assert res.record["id"] == "1" assert res.score == 0.1 + + +# -- what a Chroma failure means, and what "not there" means -- +# +# chromadb changed the error for a missing collection inside the supported range +# (chromadb >= 0.5, < 1.6): 0.5 raised ValueError, 1.0 raises errors.NotFoundError, +# which is not a ValueError. collection_exists caught bare Exception, so every failure +# read as "absent"; ensure_collection_deleted caught only ValueError, so on a modern +# chromadb a genuinely absent collection raised instead of being tolerated. + + +async def test_collection_exists_is_false_when_chroma_says_not_found(chroma_collection, mock_client): + mock_client.get_collection.side_effect = _not_found() + + assert await chroma_collection.collection_exists() is False + + +async def test_collection_exists_raises_when_the_server_cannot_be_reached(chroma_collection, mock_client): + """False means the collection is not there. A refused connection must not say that.""" + mock_client.get_collection.side_effect = ConnectionError("connection refused") + + with pytest.raises(ConnectionError): + await chroma_collection.collection_exists() + + +async def test_ensure_collection_deleted_tolerates_an_absent_collection(chroma_collection, mock_client): + """Deleting something that is not there is the case this method already meant to allow.""" + mock_client.delete_collection.side_effect = _not_found() + + await chroma_collection.ensure_collection_deleted() + + +async def test_ensure_collection_deleted_still_raises_on_a_real_failure(chroma_collection, mock_client): + mock_client.delete_collection.side_effect = ConnectionError("connection refused") + + with pytest.raises(VectorStoreOperationException): + await chroma_collection.ensure_collection_deleted() From 01b9b439763444b4807f89d74cf8e7ce26a716e1 Mon Sep 17 00:00:00 2001 From: Eric Del Orefice <114540347+ericdelorefice@users.noreply.github.com> Date: Wed, 30 Sep 2026 12:24:47 -0400 Subject: [PATCH 2/3] Catch only the dedicated exception where Chroma provides one Review point taken. Keeping ValueError in the tuple alongside NotFoundError reinstated the broad-catch symptom on modern Chroma in a narrower form, and in ensure_collection_deleted it is worse than untidy: an unrelated ValueError would be logged as "could not be deleted because it doesn't exist" and the delete silently skipped. That is the defect this PR is about, introduced by the fix for it. COLLECTION_NOT_FOUND is now (NotFoundError,) where the import succeeds and (ValueError,) only on chromadb < 1.0, where ValueError was the signal. Two tests pin it, skipped on chromadb < 1.0 where a ValueError genuinely does mean absent. Against this branch's previous commit both fail, the second with the captured log showing the swallow. Against upstream all four error-path tests fail, two per symptom. --- python/semantic_kernel/connectors/chroma.py | 4 +- .../unit/connectors/memory/test_chroma.py | 38 ++++++++++++++++--- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/python/semantic_kernel/connectors/chroma.py b/python/semantic_kernel/connectors/chroma.py index 3b5dac4b5b06..18bcae5812d7 100644 --- a/python/semantic_kernel/connectors/chroma.py +++ b/python/semantic_kernel/connectors/chroma.py @@ -15,8 +15,8 @@ try: # chromadb >= 1.0 raises NotFoundError for a missing collection from chromadb.errors import NotFoundError as _ChromaNotFoundError - COLLECTION_NOT_FOUND: tuple[type[Exception], ...] = (ValueError, _ChromaNotFoundError) -except ImportError: # chromadb < 1.0 raised ValueError + COLLECTION_NOT_FOUND: tuple[type[Exception], ...] = (_ChromaNotFoundError,) +except ImportError: # chromadb < 1.0 had no NotFoundError and raised ValueError COLLECTION_NOT_FOUND = (ValueError,) from semantic_kernel.connectors.ai.embedding_generator_base import EmbeddingGeneratorBase diff --git a/python/tests/unit/connectors/memory/test_chroma.py b/python/tests/unit/connectors/memory/test_chroma.py index e257eea4cff0..d4af3177bf8d 100644 --- a/python/tests/unit/connectors/memory/test_chroma.py +++ b/python/tests/unit/connectors/memory/test_chroma.py @@ -9,6 +9,14 @@ from semantic_kernel.connectors.chroma import ChromaCollection, ChromaStore from semantic_kernel.exceptions.vector_store_exceptions import VectorStoreOperationException +try: # mirrors the connector, resolved here so these tests do not depend on it + from chromadb.errors import NotFoundError as _ChromaNotFoundError + + COLLECTION_NOT_FOUND_IS_VALUE_ERROR = False +except ImportError: # chromadb < 1.0 + _ChromaNotFoundError = ValueError + COLLECTION_NOT_FOUND_IS_VALUE_ERROR = True + def _not_found() -> Exception: """The error Chroma raises for a missing collection, on whichever version is installed. @@ -16,11 +24,7 @@ def _not_found() -> Exception: Resolved here rather than imported from the connector, so these tests run unchanged against the code before and after the fix. """ - try: - from chromadb.errors import NotFoundError # chromadb >= 1.0 - except ImportError: # chromadb < 1.0 raised ValueError - return ValueError("Collection test_collection does not exist.") - return NotFoundError("Collection test_collection does not exist.") + return _ChromaNotFoundError("Collection test_collection does not exist.") @pytest.fixture @@ -164,3 +168,27 @@ async def test_ensure_collection_deleted_still_raises_on_a_real_failure(chroma_c with pytest.raises(VectorStoreOperationException): await chroma_collection.ensure_collection_deleted() + + +@pytest.mark.skipif( + COLLECTION_NOT_FOUND_IS_VALUE_ERROR, + reason="on chromadb < 1.0 a ValueError IS the missing-collection signal", +) +async def test_collection_exists_raises_on_an_unrelated_value_error(chroma_collection, mock_client): + """Where Chroma has a dedicated NotFoundError, a ValueError is not 'absent'.""" + mock_client.get_collection.side_effect = ValueError("some other problem") + + with pytest.raises(ValueError): + await chroma_collection.collection_exists() + + +@pytest.mark.skipif( + COLLECTION_NOT_FOUND_IS_VALUE_ERROR, + reason="on chromadb < 1.0 a ValueError IS the missing-collection signal", +) +async def test_ensure_collection_deleted_does_not_swallow_an_unrelated_value_error(chroma_collection, mock_client): + """The more dangerous half: a swallowed ValueError here would silently skip a delete.""" + mock_client.delete_collection.side_effect = ValueError("some other problem") + + with pytest.raises(VectorStoreOperationException): + await chroma_collection.ensure_collection_deleted() From 805a7864bb592c88c680dd768530026307556340 Mon Sep 17 00:00:00 2001 From: Eric Del Orefice <114540347+ericdelorefice@users.noreply.github.com> Date: Tue, 6 Oct 2026 20:33:06 -0400 Subject: [PATCH 3/3] Python: chroma - catch NotFoundError directly and wrap other errors The connector only imports on chromadb 1.x (it needs chromadb.api.collection_configuration), so the ValueError fallback for chromadb < 1.0 could never run; it is removed, with the two tests it gated. collection_exists now wraps any other error in VectorStoreOperationException, matching ensure_collection_deleted here and the Cosmos NoSQL change in #14515. --- python/semantic_kernel/connectors/chroma.py | 21 ++++++----- .../unit/connectors/memory/test_chroma.py | 36 +++++-------------- 2 files changed, 19 insertions(+), 38 deletions(-) diff --git a/python/semantic_kernel/connectors/chroma.py b/python/semantic_kernel/connectors/chroma.py index 18bcae5812d7..47d1d4e6ec73 100644 --- a/python/semantic_kernel/connectors/chroma.py +++ b/python/semantic_kernel/connectors/chroma.py @@ -11,13 +11,7 @@ from chromadb.api.collection_configuration import CreateCollectionConfiguration, CreateHNSWConfiguration from chromadb.api.types import EmbeddingFunction, Space from chromadb.config import Settings - -try: # chromadb >= 1.0 raises NotFoundError for a missing collection - from chromadb.errors import NotFoundError as _ChromaNotFoundError - - COLLECTION_NOT_FOUND: tuple[type[Exception], ...] = (_ChromaNotFoundError,) -except ImportError: # chromadb < 1.0 had no NotFoundError and raised ValueError - COLLECTION_NOT_FOUND = (ValueError,) +from chromadb.errors import NotFoundError from semantic_kernel.connectors.ai.embedding_generator_base import EmbeddingGeneratorBase from semantic_kernel.data.vector import ( @@ -138,14 +132,19 @@ async def collection_exists(self, **kwargs: Any) -> bool: """Check if the collection exists. Returns False only when Chroma says the collection is not there. Anything else - - an unreachable server, a rejected request - is raised, because the caller cannot - tell it apart from an absent collection otherwise. + an unreachable server, a rejected request - is raised as a + VectorStoreOperationException, because the caller cannot tell it apart from an + absent collection otherwise. """ try: self.client.get_collection(name=self.collection_name, embedding_function=self.embedding_func) return True - except COLLECTION_NOT_FOUND: + except NotFoundError: return False + except Exception as e: + raise VectorStoreOperationException( + f"Failed to check whether collection {self.collection_name} exists with error: {e}" + ) from e @override async def ensure_collection_exists(self, **kwargs: Any) -> None: @@ -196,7 +195,7 @@ async def ensure_collection_deleted(self, **kwargs: Any) -> None: """Delete the collection.""" try: self.client.delete_collection(name=self.collection_name) - except COLLECTION_NOT_FOUND: + except NotFoundError: logger.info(f"Collection {self.collection_name} could not be deleted because it doesn't exist.") except Exception as e: raise VectorStoreOperationException( diff --git a/python/tests/unit/connectors/memory/test_chroma.py b/python/tests/unit/connectors/memory/test_chroma.py index d4af3177bf8d..38e8b3df770b 100644 --- a/python/tests/unit/connectors/memory/test_chroma.py +++ b/python/tests/unit/connectors/memory/test_chroma.py @@ -5,26 +5,15 @@ import pytest from chromadb.api import ClientAPI from chromadb.api.models.Collection import Collection +from chromadb.errors import NotFoundError from semantic_kernel.connectors.chroma import ChromaCollection, ChromaStore from semantic_kernel.exceptions.vector_store_exceptions import VectorStoreOperationException -try: # mirrors the connector, resolved here so these tests do not depend on it - from chromadb.errors import NotFoundError as _ChromaNotFoundError - - COLLECTION_NOT_FOUND_IS_VALUE_ERROR = False -except ImportError: # chromadb < 1.0 - _ChromaNotFoundError = ValueError - COLLECTION_NOT_FOUND_IS_VALUE_ERROR = True - def _not_found() -> Exception: - """The error Chroma raises for a missing collection, on whichever version is installed. - - Resolved here rather than imported from the connector, so these tests run unchanged - against the code before and after the fix. - """ - return _ChromaNotFoundError("Collection test_collection does not exist.") + """The error Chroma raises for a missing collection.""" + return NotFoundError("Collection test_collection does not exist.") @pytest.fixture @@ -135,9 +124,9 @@ async def test_chroma_collection_search(chroma_collection, mock_client, include_ # -- what a Chroma failure means, and what "not there" means -- # -# chromadb changed the error for a missing collection inside the supported range -# (chromadb >= 0.5, < 1.6): 0.5 raised ValueError, 1.0 raises errors.NotFoundError, -# which is not a ValueError. collection_exists caught bare Exception, so every failure +# chromadb 1.x raises errors.NotFoundError for a missing collection, which is not a +# ValueError (this connector only imports on chromadb 1.x: it needs +# chromadb.api.collection_configuration). collection_exists caught bare Exception, so every failure # read as "absent"; ensure_collection_deleted caught only ValueError, so on a modern # chromadb a genuinely absent collection raised instead of being tolerated. @@ -152,8 +141,9 @@ async def test_collection_exists_raises_when_the_server_cannot_be_reached(chroma """False means the collection is not there. A refused connection must not say that.""" mock_client.get_collection.side_effect = ConnectionError("connection refused") - with pytest.raises(ConnectionError): + with pytest.raises(VectorStoreOperationException) as raised: await chroma_collection.collection_exists() + assert isinstance(raised.value.__cause__, ConnectionError) async def test_ensure_collection_deleted_tolerates_an_absent_collection(chroma_collection, mock_client): @@ -170,22 +160,14 @@ async def test_ensure_collection_deleted_still_raises_on_a_real_failure(chroma_c await chroma_collection.ensure_collection_deleted() -@pytest.mark.skipif( - COLLECTION_NOT_FOUND_IS_VALUE_ERROR, - reason="on chromadb < 1.0 a ValueError IS the missing-collection signal", -) async def test_collection_exists_raises_on_an_unrelated_value_error(chroma_collection, mock_client): """Where Chroma has a dedicated NotFoundError, a ValueError is not 'absent'.""" mock_client.get_collection.side_effect = ValueError("some other problem") - with pytest.raises(ValueError): + with pytest.raises(VectorStoreOperationException): await chroma_collection.collection_exists() -@pytest.mark.skipif( - COLLECTION_NOT_FOUND_IS_VALUE_ERROR, - reason="on chromadb < 1.0 a ValueError IS the missing-collection signal", -) async def test_ensure_collection_deleted_does_not_swallow_an_unrelated_value_error(chroma_collection, mock_client): """The more dangerous half: a swallowed ValueError here would silently skip a delete.""" mock_client.delete_collection.side_effect = ValueError("some other problem")