-
Notifications
You must be signed in to change notification settings - Fork 2k
fix(chroma): handle collection not found exception during initialization #4702
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.
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,6 +48,7 @@ | |
| import org.springframework.lang.Nullable; | ||
| import org.springframework.util.Assert; | ||
| import org.springframework.util.CollectionUtils; | ||
| import org.springframework.web.client.HttpClientErrorException; | ||
|
|
||
| /** | ||
| * {@link ChromaVectorStore} is a concrete implementation of the {@link VectorStore} | ||
|
|
@@ -117,7 +118,16 @@ public static Builder builder(ChromaApi chromaApi, EmbeddingModel embeddingModel | |
| @Override | ||
| public void afterPropertiesSet() throws Exception { | ||
| if (!this.initialized) { | ||
| var collection = this.chromaApi.getCollection(this.tenantName, this.databaseName, this.collectionName); | ||
| //if the collection doesn't exist, chromaApi.getCollection will throw an exception | ||
| ChromaApi.Collection collection = null; | ||
| try { | ||
| collection = this.chromaApi.getCollection(this.tenantName, this.databaseName, this.collectionName); | ||
| } | ||
| catch (Exception ex) { | ||
| if (!(ex.getCause() instanceof HttpClientErrorException.NotFound)) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this check be part of ChromaApi:233? If I am not mistaken, the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, I’ve looked into this further and realized the issue is in Spring AI’s own Chroma client implementation. The error message check on line 233 currently uses: String.format("Collection [%s] does not exists", collectionName)However, the actual response from the Chroma server is: Since the Chroma HTTP API returns "does not exist" ,As a result, the null-return path for missing collections is never triggered. The fix is simply to update the expected message:: if (String.format("Collection [%s] does not exist", collectionName).equals(msg)) {
return null;
}I’m happy to submit a quick PR if that would be helpful! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Changing the expected message would break compatibility with older Chroma versions. Checking the type of the exception, as you already did in this Pull Request, might be the better approach. But that is just my opinion. I am waiting for the same fix to be able to update Chroma in my applications :-) |
||
| throw ex; | ||
| } | ||
| } | ||
| if (collection == null) { | ||
| if (this.initializeSchema) { | ||
| var tenant = this.chromaApi.getTenant(this.tenantName); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exception handling logic checks
ex.getCause()forHttpClientErrorException.NotFound, but ifchromaApi.getCollection()directly throwsHttpClientErrorException.NotFound(not wrapped), this check will fail and re-throw the exception incorrectly. Consider also checking ifexitself is an instance ofHttpClientErrorException.NotFoundbefore checking the cause:if (!(ex instanceof HttpClientErrorException.NotFound || ex.getCause() instanceof HttpClientErrorException.NotFound))