Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions redisvl/utils/vectorize/bedrock.py
Original file line number Diff line number Diff line change
Expand Up @@ -193,11 +193,11 @@ def _set_model_dims(self) -> int:
# Call the protected _embed method to avoid caching this test embedding
embedding = self._embed("dimension check")
return len(embedding)
except (KeyError, IndexError) as ke:
raise ValueError(f"Unexpected response from the Bedrock API: {str(ke)}")
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for Bedrock model "
f"'{self.model}': {e}"
) from e

@retry(
wait=wait_random_exponential(min=1, max=60),
Expand Down
8 changes: 4 additions & 4 deletions redisvl/utils/vectorize/text/azureopenai.py
Original file line number Diff line number Diff line change
Expand Up @@ -208,11 +208,11 @@ def _set_model_dims(self) -> int:
# Call the protected _embed method to avoid caching this test embedding
embedding = self._embed("dimension check")
return len(embedding)
except (KeyError, IndexError) as ke:
raise ValueError(f"Unexpected response from the AzureOpenAI API: {str(ke)}")
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for Azure OpenAI deployment "
f"'{self.model}': {e}"
) from e

@deprecated_argument("text", "content")
@retry(
Expand Down
8 changes: 4 additions & 4 deletions redisvl/utils/vectorize/text/cohere.py
Original file line number Diff line number Diff line change
Expand Up @@ -166,11 +166,11 @@ def _set_model_dims(self) -> int:
# Call the protected _embed method to avoid caching this test embedding
embedding = self._embed("dimension check", input_type="search_document")
return len(embedding)
except (KeyError, IndexError) as ke:
raise ValueError(f"Unexpected response from the Cohere API: {str(ke)}")
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for Cohere model "
f"'{self.model}': {e}"
) from e

def _get_cohere_embedding_type(self, dtype: str) -> list[str]:
"""
Expand Down
37 changes: 34 additions & 3 deletions redisvl/utils/vectorize/text/huggingface.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,16 +113,47 @@ def _initialize_client(self, model: str, **kwargs):
"Please install with `pip install sentence-transformers`"
)

self._client = SentenceTransformer(model, **kwargs)
try:
self._client = SentenceTransformer(model, **kwargs)
except OSError as e:
# This is where a bad model name or path actually surfaces --
# _set_model_dims() never sees it, since loading happens here,
# before that method's try block runs.
raise ValueError(
f"Could not load the local embedding model '{model}'. Check the "
f"model name or path and that the model has been downloaded: {str(e)}"
) from e
Comment on lines +118 to +125

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OSError is wider than it looks. huggingface_hub's GatedRepoError, LocalEntryNotFoundError, OfflineModeIsEnabled, and HfHubHTTPError all subclass it, and so does requests.exceptions.RequestException. So a gated model with a perfectly correct name, or a proxy blocking huggingface.co, both get told to check the model name and whether it's downloaded. RepositoryNotFoundError is the only one that advice actually fits.

Either catch RepositoryNotFoundError specifically, or widen the wording to cover the rest, along the lines of "check the model name or path, your network access to huggingface.co, and whether the repo requires accepting a licence".

except RuntimeError as e:
raise ValueError(
f"The local embedding model '{model}' failed to load. On CUDA this is "
f"commonly an out-of-memory or device mismatch -- retry with "
f"device='cpu': {str(e)}"
) from e

def _set_model_dims(self):
try:
embedding = self._embed("dimension check")
except (KeyError, IndexError) as ke:
raise ValueError(f"Empty response from the embedding model: {str(ke)}")
except OSError as e:
# Unlike _initialize_client()'s OSError handler, this one fires
# after SentenceTransformer already loaded successfully -- the
# failure is in the dimension probe/encode call, not the load.
raise ValueError(
f"The local embedding model '{self.model}' failed while determining its "
f"dimensions: {str(e)}"
) from e
Comment thread
cursor[bot] marked this conversation as resolved.
Comment thread
cursor[bot] marked this conversation as resolved.
except RuntimeError as e:
raise ValueError(
f"The local embedding model '{self.model}' failed while determining its "
f"dimensions. On CUDA this is commonly an out-of-memory or device "
f"mismatch -- retry with device='cpu': {str(e)}"
) from e
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for local model "
f"'{self.model}': {str(e)}"
) from e
return len(embedding)

@deprecated_argument("text", "content")
Expand Down
8 changes: 4 additions & 4 deletions redisvl/utils/vectorize/text/mistral.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,11 @@ def _set_model_dims(self) -> int:
# Call the protected _embed method to avoid caching this test embedding
embedding = self._embed("dimension check")
return len(embedding)
except (KeyError, IndexError) as ke:
raise ValueError(f"Unexpected response from the MISTRAL API: {str(ke)}")
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for Mistral model "
f"'{self.model}': {e}"
) from e

@deprecated_argument("text", "content")
@retry(
Expand Down
8 changes: 4 additions & 4 deletions redisvl/utils/vectorize/text/openai.py
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,11 @@ def _set_model_dims(self) -> int:
# Use the parent embed() method which handles caching

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Small one while you're here: this comment says the opposite of the code. embed() is the caching path and _embed() bypasses it, which is what you want for a throwaway probe. The other seven vectorizers say "Call the protected _embed method to avoid caching this test embedding". Could you match them?

embedding = self._embed("dimension check")
return len(embedding)
except (KeyError, IndexError) as ke:
raise ValueError(f"Unexpected response from the OpenAI API: {str(ke)}")
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for OpenAI model "
f"'{self.model}': {e}"
) from e

@deprecated_argument("text", "content")
@retry(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This decorator is what swallows the cause. retry_if_not_exception_type(TypeError) doesn't exempt the ValueError that _embed raises, so six attempts run and tenacity wraps the result in RetryError. Adding reraise=True here lets the original propagate, and your new message then shows the real reason.

Same change needed in mistral.py, bedrock.py, and on voyageai.py's _embed_many decorator. azureopenai.py:214 already has it if you want a reference.

Expand Down
8 changes: 4 additions & 4 deletions redisvl/utils/vectorize/vertexai.py
Original file line number Diff line number Diff line change
Expand Up @@ -232,11 +232,11 @@ def _set_model_dims(self) -> int:
# Call the protected _embed method to avoid caching this test embedding
embedding = self._embed("dimension check")
return len(embedding)
except (KeyError, IndexError) as ke:
raise ValueError(f"Unexpected response from the VertexAI API: {str(ke)}")
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for Vertex AI model "
f"'{self.model}': {e}"
) from e

@retry(
wait=wait_random_exponential(min=1, max=60),
Expand Down
8 changes: 4 additions & 4 deletions redisvl/utils/vectorize/voyageai.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,11 +234,11 @@ def _set_model_dims(self) -> int:
# Call the protected _embed method to avoid caching this test embedding
embedding = self._embed("dimension check", input_type="document")
return len(embedding)
except (KeyError, IndexError) as ke:
raise ValueError(f"Unexpected response from the VoyageAI API: {str(ke)}")
except Exception as e: # pylint: disable=broad-except
# fall back (TODO get more specific)
raise ValueError(f"Error setting embedding model dimensions: {str(e)}")
raise ValueError(
f"Error setting embedding model dimensions for VoyageAI model "
f"'{self.model}': {e}"
) from e

def _get_batch_size(self) -> int:
"""
Expand Down
227 changes: 227 additions & 0 deletions tests/unit/test_vectorizer_dim_errors.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,227 @@
"""Errors from vectorizer ``_set_model_dims()`` become an actionable ``ValueError``.

Each vectorizer probes its provider with a throwaway embedding call to learn the
model's dimensionality. When that probe fails, ``_set_model_dims()`` wraps
whatever it catches in a ``ValueError`` that names the provider and the model,
chained with ``from e`` so the original exception (SDK error, retry exhaustion,
etc.) stays visible in the traceback rather than being swallowed.
"""

from unittest.mock import MagicMock, patch

import httpx
import pytest


def _openai_error(cls, status):
"""Build an openai SDK error without performing a request."""
request = httpx.Request("POST", "https://api.openai.com/v1/embeddings")
response = httpx.Response(status, request=request)
return cls("boom", response=response, body=None)


# ---------------------------------------------------------------------------
# End-to-end: the real _embed()/_initialize_client() code runs.
# ---------------------------------------------------------------------------


def test_openai_dim_probe_names_the_model_and_chains_the_cause(monkeypatch):
"""OpenAI's real client.embeddings.create() raises, _embed() wraps it, and
_set_model_dims() must still report the model and preserve the cause."""
import time

import openai

from redisvl.utils.vectorize.text.openai import OpenAITextVectorizer

# _embed() is @retry-decorated and does not exempt ValueError from retrying,
# so a permanent failure like a 401 is retried up to 6 times with exponential
# backoff before RetryError is raised. Skip the real sleeps -- the retrying
# itself isn't what this test is checking.
monkeypatch.setattr(time, "sleep", lambda *a, **k: None)

error = _openai_error(openai.AuthenticationError, 401)
mock_client = MagicMock()
mock_client.embeddings.create.side_effect = error

with patch.object(
OpenAITextVectorizer,
"_initialize_clients",
lambda self, *a, **k: setattr(self, "_client", mock_client),
):
with pytest.raises(ValueError) as excinfo:
OpenAITextVectorizer(model="text-embedding-3-small")

message = str(excinfo.value)
assert "text-embedding-3-small" in message
assert excinfo.value.__cause__ is not None


def test_bedrock_dim_probe_names_the_model_and_chains_the_cause(monkeypatch):
"""Bedrock's real client.invoke_model() raises a ClientError, _embed() wraps
it, and _set_model_dims() must still report the model and preserve the cause."""
import time

from botocore.exceptions import ClientError

from redisvl.utils.vectorize.bedrock import BedrockVectorizer

monkeypatch.setattr(time, "sleep", lambda *a, **k: None)

denied = ClientError(
{"Error": {"Code": "AccessDeniedException", "Message": "nope"}}, "InvokeModel"
)
mock_client = MagicMock()
mock_client.invoke_model.side_effect = denied

with patch.object(
BedrockVectorizer,
"_initialize_client",
lambda self, *a, **k: setattr(self, "_client", mock_client),
):
with pytest.raises(ValueError) as excinfo:
BedrockVectorizer(model="amazon.titan-embed-text-v2:0")

message = str(excinfo.value)
assert "amazon.titan-embed-text-v2:0" in message
assert excinfo.value.__cause__ is not None


def test_huggingface_dim_probe_reports_local_model_load_failure():
"""A HuggingFace model that fails to load raises OSError inside the real
SentenceTransformer() construction, in _initialize_client() -- before
_set_model_dims() ever runs. This must be caught where it actually happens."""
from redisvl.utils.vectorize.text.huggingface import HFTextVectorizer

with patch(
"sentence_transformers.SentenceTransformer",
side_effect=OSError("no such file"),
):
with pytest.raises(ValueError) as excinfo:
HFTextVectorizer(model="sentence-transformers/all-mpnet-base-v2")

message = str(excinfo.value)
assert "sentence-transformers/all-mpnet-base-v2" in message
assert "downloaded" in message


# ---------------------------------------------------------------------------
# Wrap-and-chain: _embed() is patched to raise directly, pinning that
# _set_model_dims() names the provider/model and chains the real cause via
# `from e` rather than losing it.
# ---------------------------------------------------------------------------


@pytest.mark.parametrize(
"vectorizer_path, class_name, init_method, model",
[
(
"redisvl.utils.vectorize.text.azureopenai",
"AzureOpenAITextVectorizer",
"_initialize_clients",
"my-deployment",
),
(
"redisvl.utils.vectorize.text.cohere",
"CohereTextVectorizer",
"_initialize_client",
"embed-english-v3.0",
),
(
"redisvl.utils.vectorize.text.mistral",
"MistralAITextVectorizer",
"_initialize_client",
"mistral-embed",
),
(
"redisvl.utils.vectorize.vertexai",
"VertexAIVectorizer",
"_initialize_client",
"text-embedding-004",
),
],
)
def test_dim_probe_names_the_model_and_chains_the_cause(
vectorizer_path, class_name, init_method, model
):
import importlib

module = importlib.import_module(vectorizer_path)
vectorizer_cls = getattr(module, class_name)

cause = RuntimeError("boom from the SDK")

with patch.object(vectorizer_cls, init_method, lambda self, *a, **k: None):
with patch.object(vectorizer_cls, "_embed", side_effect=cause):
with pytest.raises(ValueError) as excinfo:
vectorizer_cls(model=model)

message = str(excinfo.value)
assert model in message
assert excinfo.value.__cause__ is cause


def test_voyageai_dim_probe_names_the_model_and_chains_the_cause():
from redisvl.utils.vectorize.voyageai import VoyageAIVectorizer

cause = RuntimeError("boom from the SDK")

def _fake_init(self, *a, **k):
# _setup() reaches into self._client / self._aclient right after
# _initialize_client() returns (to grab .embed / .multimodal_embed), so
# the no-op stub has to leave both set rather than leaving them unset.
self._client = MagicMock()
self._aclient = MagicMock()

with patch.object(VoyageAIVectorizer, "_initialize_client", _fake_init):
with patch.object(VoyageAIVectorizer, "_embed", side_effect=cause):
with pytest.raises(ValueError) as excinfo:
VoyageAIVectorizer(model="voyage-3")

message = str(excinfo.value)
assert "voyage-3" in message
assert excinfo.value.__cause__ is cause


def test_voyageai_dim_probe_catches_bad_model_id_type_error():
"""VoyageAI's _embed_many() re-raises InvalidRequestError as TypeError --
deliberately, so retry_if_not_exception_type(TypeError) skips retrying it,
since a bad model id can never succeed no matter how many attempts. This
drives the real _embed_many() code (only the client's .embed() call is
stubbed) to prove the TypeError path is still caught by the generic
except Exception clause."""
import voyageai.error

from redisvl.utils.vectorize.voyageai import VoyageAIVectorizer

bad_model = voyageai.error.InvalidRequestError("model not found")
mock_client = MagicMock()
mock_client.embed.side_effect = bad_model

def _fake_init(self, *a, **k):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This test passes without testing anything. _fake_init doesn't set _token_batching_supported, which the real _initialize_client sets and _embed_many reads, so construction dies on AttributeError before your stubbed .embed() is ever called: mock_client.embed.call_count is 0. The InvalidRequestError to TypeError path your docstring describes never runs, and the assertions on lines 210-211 are weak enough not to catch it.

It also costs about 18 seconds, because AttributeError isn't TypeError so it retries six times with real backoff. Setting self._token_batching_supported = True here, plus the monkeypatch.setattr(time, "sleep", ...) guard your OpenAI and Bedrock tests use on lines 41 and 69, fixes both.

self._client = mock_client
self._aclient = mock_client

with patch.object(VoyageAIVectorizer, "_initialize_client", _fake_init):
with pytest.raises(ValueError) as excinfo:
VoyageAIVectorizer(model="not-a-real-voyage-model")

message = str(excinfo.value)
assert "not-a-real-voyage-model" in message
assert excinfo.value.__cause__ is not None


def test_unanticipated_errors_still_become_valueerror():
"""The generic fallback must survive: no error may escape raw."""
from redisvl.utils.vectorize.text.openai import OpenAITextVectorizer

with patch.object(
OpenAITextVectorizer, "_initialize_clients", lambda self, *a, **k: None
):
with patch.object(
OpenAITextVectorizer, "_embed", side_effect=ZeroDivisionError("surprise")
):
with pytest.raises(ValueError) as excinfo:
OpenAITextVectorizer(model="text-embedding-3-small")

assert "text-embedding-3-small" in str(excinfo.value)
Loading