From 43f13bcab8a2c18f4bd5dbd29f277ed605bb1844 Mon Sep 17 00:00:00 2001 From: Hammad Bashir Date: Tue, 29 Oct 2024 13:22:16 -0700 Subject: [PATCH] [CLN] make ValueError and TypeError present as InvalidArgumentError (#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 --- chromadb/errors.py | 1 + chromadb/server/fastapi/__init__.py | 10 ++++++++++ clients/js/src/Errors.ts | 9 +++++++++ clients/js/test/get.collection.test.ts | 4 ++-- 4 files changed, 22 insertions(+), 2 deletions(-) diff --git a/chromadb/errors.py b/chromadb/errors.py index ab0c7d1b496..6f728e6ea52 100644 --- a/chromadb/errors.py +++ b/chromadb/errors.py @@ -182,6 +182,7 @@ def name(cls) -> str: error_types: Dict[str, Type[ChromaError]] = { "InvalidDimension": InvalidDimensionException, + "InvalidArgumentError": InvalidArgumentError, "InvalidCollection": InvalidCollectionException, "IDAlreadyExists": IDAlreadyExistsError, "DuplicateID": DuplicateIDError, diff --git a/chromadb/server/fastapi/__init__.py b/chromadb/server/fastapi/__init__.py index a41f3913385..da3babf5a48 100644 --- a/chromadb/server/fastapi/__init__.py +++ b/chromadb/server/fastapi/__init__.py @@ -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) diff --git a/clients/js/src/Errors.ts b/clients/js/src/Errors.ts index d3a077cd9f9..1aa69720004 100644 --- a/clients/js/src/Errors.ts +++ b/clients/js/src/Errors.ts @@ -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) { @@ -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; } diff --git a/clients/js/test/get.collection.test.ts b/clients/js/test/get.collection.test.ts index e4e7de6d427..e30d79488a2 100644 --- a/clients/js/test/get.collection.test.ts +++ b/clients/js/test/get.collection.test.ts @@ -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"; @@ -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"`, );