Skip to content

Commit

Permalink
[CLN] make ValueError and TypeError present as InvalidArgumentError (#…
Browse files Browse the repository at this point in the history
…3017)

## Description of changes

*Summarize the changes made by this PR.*
 - Improvements & Bug fixes
	 - This will propagate value/type errors as invalid argument errors
 - New functionality
	 - None

## Test plan
*How are these changes tested?*
- [x] Tests pass locally with `pytest` for python, `yarn test` for js,
`cargo test` for rust

## Documentation Changes
None
  • Loading branch information
HammadB authored Oct 29, 2024
1 parent b481763 commit 43f13bc
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 2 deletions.
1 change: 1 addition & 0 deletions chromadb/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ def name(cls) -> str:

error_types: Dict[str, Type[ChromaError]] = {
"InvalidDimension": InvalidDimensionException,
"InvalidArgumentError": InvalidArgumentError,
"InvalidCollection": InvalidCollectionException,
"IDAlreadyExists": IDAlreadyExistsError,
"DuplicateID": DuplicateIDError,
Expand Down
10 changes: 10 additions & 0 deletions chromadb/server/fastapi/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ async def catch_exceptions_middleware(
return await call_next(request)
except ChromaError as e:
return fastapi_json_response(e)
except ValueError as e:
return JSONResponse(
content={"error": "InvalidArgumentError", "message": str(e)},
status_code=400,
)
except TypeError as e:
return JSONResponse(
content={"error": "InvalidArgumentError", "message": str(e)},
status_code=400,
)
except Exception as e:
logger.exception(e)
return JSONResponse(content={"error": repr(e)}, status_code=500)
Expand Down
9 changes: 9 additions & 0 deletions clients/js/src/Errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ export class InvalidCollectionError extends Error {
}
}

export class InvalidArgumentError extends Error {
name = "InvalidArgumentError";
constructor(message: string, public readonly cause?: unknown) {
super(message);
}
}

export class ChromaUniqueError extends Error {
name = "ChromaUniqueError";
constructor(message: string, public readonly cause?: unknown) {
Expand All @@ -82,6 +89,8 @@ export function createErrorByType(type: string, message: string) {
switch (type) {
case "InvalidCollection":
return new InvalidCollectionError(message);
case "InvalidArgumentError":
return new InvalidArgumentError(message);
default:
return undefined;
}
Expand Down
4 changes: 2 additions & 2 deletions clients/js/test/get.collection.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import {
test,
} from "@jest/globals";
import { DOCUMENTS, EMBEDDINGS, IDS, METADATAS } from "./data";
import { ChromaValueError, InvalidCollectionError } from "../src/Errors";
import { InvalidArgumentError, InvalidCollectionError } from "../src/Errors";
import { DefaultEmbeddingFunction } from "../src/embeddings/DefaultEmbeddingFunction";
import { StartedTestContainer } from "testcontainers";
import { ChromaClient } from "../src/ChromaClient";
Expand Down Expand Up @@ -65,7 +65,7 @@ describe("get collections", () => {
});
} catch (error: any) {
expect(error).toBeDefined();
expect(error).toBeInstanceOf(ChromaValueError);
expect(error).toBeInstanceOf(InvalidArgumentError);
expect(error.message).toMatchInlineSnapshot(
`"Expected where operator to be one of $gt, $gte, $lt, $lte, $ne, $eq, $in, $nin, got $contains"`,
);
Expand Down

0 comments on commit 43f13bc

Please sign in to comment.