-
Notifications
You must be signed in to change notification settings - Fork 38
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
[Bug] Breaking change for Bun runtime between v1.0.1 and v1.1.1 #141
Comments
Thank you for the detailed bug report. I will try to look at this soon and add some bun integration tests so this doesn't happen again. |
Hello, are there any updates on this? |
any updates on this? |
## Problem Bun has been a rapidly emerging toolkit for developing, testing, running, and bundling Javascript and TypeScript. We currently have an outstanding GH issue related to the bun runtime: #141. It would be nice to add some coverage in CI for running our integration tests with bun as the runner. @jhamon kicked this work off a while ago and I wanted to make sure we got this included. ## Solution - Update `/github/workflows/testing.yml` and add a new `config` section to the `integration-tests` matrix. Tests will now be run with `npm` in node and edge, and `bun` with node for bun versions `1.0.4` -> `1.0.7`. ## Type of Change - [X] Infrastructure change (CI configs, etc) ## Test Plan Make sure all the integration tests are 🟢 in CI.
## Problem Bun has been a rapidly emerging toolkit for developing, testing, running, and bundling Javascript and TypeScript. We currently have an outstanding GH issue related to the bun runtime: #141. It would be nice to add some coverage in CI for running our integration tests with bun as the runner. @jhamon kicked this work off a while ago and I wanted to make sure we got this included. ## Solution - Update `/github/workflows/testing.yml` and add a new `config` section to the `integration-tests` matrix. Tests will now be run with `npm` in node and edge, and `bun` with node for bun versions `1.0.4` -> `1.0.7`. ## Type of Change - [X] Infrastructure change (CI configs, etc) ## Test Plan Make sure all the integration tests are 🟢 in CI.
Still broken :'( |
Confirming this is still broken here as well. |
gg still broken on 1.1.18 |
+1 |
Still broken Any updates or workarounds? |
My workaround for the time being is to use Cohere (or Elastic Search): https://cohere.com |
Is this a new bug?
Current Behavior
Consider the following code:
When we run this script with
node index.js
we have no problems. However, when we run it withbun index.js
it throws an error:Expected Behavior
It should not throw an error when working with Bun.
To go further, I have edited the code at the point of error to see exactly what is going on. Beginning at line 502 I have modified it as:
which should do the same thing as
this.raw.json()
but allow us to see the body before throwing an error. This way, we see that with Bun we print the following within thecatch
function:In other words, something causes the body to be empty string, which of course can't be parsed to JSON.
This issue does not exist for pinecone client v.1.0.1.
Steps To Reproduce
node
and confirm it works.bun
and confirm it throws an error.It seems to be important that
includeValues
is set to betrue
.Relevant log output
No response
Environment
Additional Context
Here is a shortcut to Bun issues page: https://github.com/oven-sh/bun/issues
They have quite a lot of issues, but if we can pinpoint what causes the issue with Pinecone we can link to a relevant issue within Bun. Perhaps a different method was used to parse the request body between v1.0.1 ~ v.1.1.1 and Bun does not support the latter?
Mentioning my teammate @anilaltuner to also notify him & for his help on finding the bug.
The text was updated successfully, but these errors were encountered: