[Bug] v1 Index<T>.query throwing TypeError: Cannot read properties of undefined (reading 'text') during migration #124
Comments
|
FWIW, this works in place of const url = `https://${process.env.PINECONE_HOST!}/query`;
const res = await fetch(url, {
method: "POST",
headers: {
accept: "application/json",
"Content-Type": "application/json",
"Api-Key": process.env.PINECONE_API_KEY || "unknown",
},
body: JSON.stringify({
includeValues: false,
includeMetadata: true,
topK: topK,
vector: embeddings,
}),
});
const resJson = await res.json();
return resJson; |
|
getting the same error? the above solution didn't work for me. |
|
I'm getting the same error as well. Using the config laid out from the read me |
|
Sorry all for the trouble. Looking into it now. |
|
@jhamon any luck here? |
const url = `${process.env.PINECONE_HOST!}/query`;
const res = await fetch(url, {
method: "POST",
headers: {
accept: "application/json",
"Content-Type": "application/json",
"Api-Key": process.env.PINECONE_API_KEY || "unknown",
},
body: JSON.stringify({
includeValues: false,
includeMetadata: true,
topK: <topK>,
vector: embeddings,
namespace: <namespace>,
}),
});
const resJson = await res.json() as QueryResponse<RecordMetadata>;This worked for me. |
|
Is anyone seeing this error outside of a next.js context? My working theory is that the fetch implementation is missing/non-functional due to next.js stubbing out the cross-fetch polyfill we rely on. But if people are also seeing this outside the context of a next.js app, we might be dealing with multiple unrelated issues that result in similar errors. |
## Problem Next.js seems to stub out the cross-fetch polyfill which has the effect of silently breaking code which depends on cross-fetch to make API requests. Related bugs: - #124 - v1 migration problems in pinecone-io/pinecone-vercel-starter#14 ## Solution I have added a `getFetch` utility which looks for a `fetch` implementation already loaded in the global scope and returns the cross-fetch implementation only as a last resort. - Now when using our client library in a next.js project, this should result in the `@vercel/fetch` implementation being used in favor of the disabled polyfill. - When using this client library in other environments where next.js is not being used, it still activates the cross-fetch polyfill. - Also, I've exposed an experimental configuration option for anyone who wants to pass in a different fetch implementation to use. This could open up some flexibility for people with specific network configurations who need the ability to configure the http client being used in some way. If the user provides a fetch implementation through the `fetchApi` config option, it will take priority over a fetch in the global scope or the cross-fetch polyfill. ### Other changes - Added additional integration tests for `query` method - Added an explicit dependency on the `encoding` module to resolve a warning message [similar to this one](vercel/next.js#54109) ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) ## Test Plan I am currently testing this change in the context of work-in-progress to migrate our vercel-starter example to using the v1 version of the client. pinecone-io/pinecone-vercel-starter#14 - Checkout the PR branch from pinecone-vercel-starter in a sibling directory - Install my local changes to this client with `npm install ../pinecone-ts-client` - Try to run the starter project with `npm run dev` - Attempt to use the sample app and observe it progresses beyond the error related broken cross-fetch
|
I think I've solved this issue in the v1.1.0 release. Please upgrade and let us know if you're still having a problem with this. |
Updated the package but still facing the issue
|
|
@Nipunwalia08 From that stacktrace alone I'm not sure that this is a Pinecone error. Can you open a fresh issue in this repo (just to focus the discussion around your situation) and include some additional info about what you have in your |
|
@jhamon thanks for taking a look and fixing the issue in a new release! Interestingly enough, I am getting this issue (with the same stack trace as OP) when upserting. I created a simple PDF from which to debug to see if any of the created vectors contained undefined metadata that would cause the reading of the text property to error out. Just to start the conversation, would there be any resemblance between the ns.query() and the ns.upsert()'s implementation of the cross-fetch polyfill that pinecone relies on? Let me know if anything jumps out at first glance; otherwise, I will open a fresh issue with further details. |
I'm wondering if there is something happening that is tied to a specific version of nextjs, since I did a lot of hands-on testing with our The missing It seems like there is at least one other error case not covered here (i.e. FetchError and ResponseError are not the only things that can occur). I should check rather than casting here to avoid masking the real issue. |
## Problem Some users are still running into errors that can't be properly diagnosed because of a bug in error handling causing the real error to get obscured by an unrelated problem. There's a point at which we assume we should be dealing with a `ResponseError` with a `text()` field; in reality, that's not a correct assumption so it's not safe to cast to that type. - #124 ## Solution This was one of those changes where you pull on the thread and the whole carpet unravels. - Refactor error handling away from nested callbacks into a flat error handler function - Wrap the underlying exception by attaching it as a `cause` whenever a `PineconeConnectionError` exception is being thrown. This will help us investigate #124 - Migrate away from method-specific error handling to a consistent approach based on middleware functions. We already had an httpErrorMapper, but with this approach it only has to get wired up twice instead of like 20 times so that really simplifies what we need to cover in unit testing. - Implement a bunch of new integration tests to give confidence the middleware is actually working as expected. - The cost of this change is we lose a little bit of convenience in the 404 case where we were fetching and presenting the list of valid indexes/collections. But that small bit of convenience doesn't seem worth a radically more complex approach to error handling. - We no longer need to test exception handling in most method implementations - It unlocks the ability for some really cool debug options. See #136. #### Summary of files changed: - **Error handling logic** in `src/errors/handling.ts`: - Refactor from nested callbacks into a flat error handler function. - Continue to delegate to httpErrorMapper, but check that the error actually is a ResponseError before trying to access the `text()` property. - For other errors, wrap the underlying errors and throw `PineconeConnectionError` - **Error classes** in `src/errors`: - `PineconeConnectionError` and `BasePineconeError` modified to allow reference to underlying errors to be captured. - Revised error message that explains more reasons why `PineconeConnectionError` might occur. - **Middleware**: These get invoked at different points in the lifecycle of every request. - Defined middleware using `handleApiError` - Wired up new middleware for error handling in `src/pinecone.ts` and `src/data/index.ts`. Now we can be confident we are handling errors consistently across every method without the need for a lot of repetitive unit testing to verify wiring on each client method. - **New integration tests**. - Added a more robust suite of integration tests to exercise the error handling in a variety of real scenarios. - Deleted method-specific error handling that is no longer needed - Deleted method-level unit testing of error handling ## Type of Change - [x] Bug fix (non-breaking change which fixes an issue) ## Test Plan Describe specific steps for validating this change.
|
I just shipped a If anyone experiencing this problem can share full details about your environment and dependencies (ideally a minimum reproduction case), that would be very helpful. So far I have not been able to reproduce the issue in our sample projects and integration tests. Specifically it would help me to know:
I appreciate everyone's patience as we work through this. |
|
@jhamon Hello again, I'm using Sveltekit so the problem is very likely NodeJS/Vercel related, not NextJS. Pre v1.1.1 the titled error is thrown when using Vercel Serverless or Edge; both fail. Once updated to v1.1.1 the new error is as follows: error code: 525 Status: 525. (Probably a failed SSL Handshake) Thank you and let me know if theres anything I can test to help resolve this. |
|
@jhamon Any update/thoughts on this? Not being able to use Vercel is a critical hit |
|
Can we please have a progress update here? I need to know if there's anything I can do to help or if Pinecone is abandoning vercel. If so I'll need to sprint to pgvector to meet my teams release date because currently every single npm version of pincone is erroring out. Thank you |
anawishnoff
added
the
status:needs-triage
|
As this issue is almost a year old, I'll go ahead and close this as stale. However please feel free to reopen or file a new issue if you're still having trouble here - thank you! |
anawishnoff
removed
the
status:needs-triage
Is this a new bug?
Current Behavior
I'm migrating to v1 as described in https://github.com/pinecone-io/pinecone-ts-client/blob/main/v1-migration.md#query
I can upsert after migrating just fine, but I can't query. My code is as follows:
I have verified that there are actually embeddings (
RecordValues).The stack trace is a bit unhelpful:
Any ideas?
Expected Behavior
No error to be thrown by
ns.query, or, a helpful error message or stack trace.Steps To Reproduce
See current behavior code.
Relevant log output
No response
Environment
Additional Context
No response
The text was updated successfully, but these errors were encountered: