Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[Bug]: Collection.delete raises Exception instead of ValueError #2569

Closed
curious-broccoli opened this issue Jul 24, 2024 · 1 comment · Fixed by #3439
Closed

[Bug]: Collection.delete raises Exception instead of ValueError #2569

curious-broccoli opened this issue Jul 24, 2024 · 1 comment · Fixed by #3439
Assignees
@itaismith
Labels
bug Something isn't working

Comments

@curious-broccoli
Copy link

What happened?

Maybe this is intended, but the docs for Collection.delete say that it should raise ValueError if e.g. no ids are passed. But it raises an Exception.

import chromadb

client = chromadb.HttpClient(host="localhost", port="8100")
collection = client.get_collection("hpc-expert")
try:
    collection.delete(ids=[])
except ValueError:
    print("ValueError")
except Exception:
    print("Exception")
    raise

Versions

Client: chromadb 0.5.0 or chromadb-client 0.5.5.dev0, Python 3.12.4
Server: docker image chromadb/chroma:0.5.4

Relevant log output

Exception
Traceback (most recent call last):
  File "/home/moritz/programming/python/venv/chromaJustClient/lib/python3.12/site-packages/chromadb/api/base_http_client.py", line 95, in _raise_chroma_error
    resp.raise_for_status()
  File "/home/moritz/programming/python/venv/chromaJustClient/lib/python3.12/site-packages/httpx/_models.py", line 761, in raise_for_status
    raise HTTPStatusError(message, request=request, response=self)
httpx.HTTPStatusError: Server error '500 Internal Server Error' for url 'http://localhost:8100/api/v1/collections/1e2073a0-4dcf-4f38-9a09-7d40b783d5cd/delete'
For more information check: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/500

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/moritz/programming/python/llm-server-wrappers/hpc-expert/hpc-expert/testdb.py", line 6, in <module>
    collection.delete(ids=[])
  File "/home/moritz/programming/python/venv/chromaJustClient/lib/python3.12/site-packages/chromadb/api/models/Collection.py", line 332, in delete
    self._client._delete(self.id, ids, where, where_document)
  File "/home/moritz/programming/python/venv/chromaJustClient/lib/python3.12/site-packages/chromadb/telemetry/opentelemetry/__init__.py", line 146, in wrapper
    return f(*args, **kwargs)
           ^^^^^^^^^^^^^^^^^^
  File "/home/moritz/programming/python/venv/chromaJustClient/lib/python3.12/site-packages/chromadb/api/fastapi.py", line 368, in _delete
    resp_json = self._make_request(
                ^^^^^^^^^^^^^^^^^^^
  File "/home/moritz/programming/python/venv/chromaJustClient/lib/python3.12/site-packages/chromadb/api/fastapi.py", line 87, in _make_request
    BaseHTTPClient._raise_chroma_error(response)
  File "/home/moritz/programming/python/venv/chromaJustClient/lib/python3.12/site-packages/chromadb/api/base_http_client.py", line 97, in _raise_chroma_error
    raise (Exception(resp.text))
Exception: {"error":"ValueError('\\n                You must provide either ids, where, or where_document to delete. If\\n                you want to delete all data in a collection you can delete the\\n                collection itself using the delete_collection method. Or alternatively,\\n                you can get() all the relevant ids and then delete them.\\n                ')"}
@curious-broccoli curious-broccoli added the bug Something isn't working label Jul 24, 2024
@tazarov
Copy link
Contributor

tazarov commented Jul 24, 2024

Hey @curious-broccoli, thanks for bringing this to our attention. Indeed, generic value errors like this one cause the server to return a pretty generic exception, str(e), and then the client handles it as a generic Exception with raise_for_status().

We have a PR that addresses this to a certain degree, in addition to other issues - #1494.

@itaismith itaismith self-assigned this Jan 7, 2025
@itaismith itaismith mentioned this issue Jan 8, 2025
Merged
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@itaismith itaismith

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

[BUG] UniqueConstraintError not included as a ChromaError chroma-core/chroma
3 participants
@tazarov @itaismith @curious-broccoli