oven-sh / bun

Incredibly fast JavaScript runtime, bundler, test runner, and package manager – all in one
https://bun.sh
Other
74.02k stars 2.75k forks source link

Some Stripe API calls failing in production since upgrading from Bun 1.0.x to Bun 1.1.x #10975

Open nathankleyn opened 5 months ago

nathankleyn commented 5 months ago

What version of Bun is running?

1.1.7+b0b7db5c0

What platform is your computer?

Linux 5.15.60+ x86_64 unknown

What steps can reproduce the bug?

  1. Make a minimal script to setup a Stripe client and make a call to get a Stripe connection token:
const stripe = new Stripe("<some api key>");
const res = await stripe.terminal.connectionTokens.create();
  1. Deploy it within Docker container based off oven/bun:1.1.7-debian to Google Cloud Run

What is the expected behavior?

The call should work and return an object of the form:

{
  "object": "terminal.connection_token",
  "secret": "<some secret token>"
}

What do you see instead?

The call fails with an exception:

23 |  * StripeError is the base error from which all other more specific Stripe errors derive.
24 |  * Specifically for errors returned from Stripe's REST API.
25 |  */
26 | export class StripeError extends Error {
27 |     constructor(raw = {}, type = null) {
28 |         super(raw.message);
                 ^
error: Invalid JSON received from the Stripe API
 code: "undefined"
      at new StripeError (/app/node_modules/stripe/esm/Error.js:28:13)
      at new StripeAPIError (/app/node_modules/stripe/esm/Error.js:109:42)
      at /app/node_modules/stripe/esm/RequestSender.js:174:33

Caused by:

 {
  message: "Invalid JSON received from the Stripe API",
  exception: 91 |                 response += chunk;
92 |             });
93 |             this._res.once('end', () => {
94 |                 try {
95 |                     resolve(JSON.parse(response));
96 |                 }
     ^
SyntaxError: JSON Parse error: Unrecognized token '<'
      at <parse> ()
      at parse (native:1:1)
      at /app/node_modules/stripe/esm/net/NodeHttpClient.js:96:16
      at emit (native:1:1)
      at endReadableNT (node:stream:2414:27)

Additional information

The actual response from the Stripe API under the covers is:

<html>
<head><title>400 The plain HTTP request was sent to HTTPS port</title></head>
<body>
<center><h1>400 Bad Request</h1></center>
<center>The plain HTTP request was sent to HTTPS port</center>
<hr><center>nginx</center>
</body>

The URL of the API Stripe is actually calling to is to:

http://api.stripe.com:443/v1/terminal/connection_tokens

^ Note the http but the port :443.

It seems that before Bun 1.1.0, the https was being inferred because of the port (?) but on every version between 1.1.0 and 1.1.7 the http is being left causing this error upstream.

It's worth noting that I cannot seem to reproduce this issue outside of GCP β€” it only seems to happen there. If I try to run the same Docker container on Orbstack locally, or the app directly on MacOS, all works fine β€” this is mystifying as I'm trying to make it reproducible for you but have really struggled to make it easier!

Our company is encountering this issue in production which is blocking us from upgrading to Bun 1.1.x. Pinning to Bun 1.0.36 resolves the issue.

Jarred-Sumner commented 5 months ago

@nathankleyn can you give it a try in Bun v1.1.8? @nektro fixed some things in node:http which might help here (notably: adding .agent as a getter)

bun upgrade --canary
nathankleyn commented 5 months ago

@Jarred-Sumner Thanks for super fast reply! On it and will report back ASAP πŸ‘€ πŸ‘

nathankleyn commented 5 months ago

@Jarred-Sumner Just gave it a go in production with 1.1.8-canary.1+f52c17781 β€” unfortunately still the same issue with the same response as above.

Let me know if there's anything I can do to help make this more easily reproducible or test anything at all, we're at your disposal as I know the case above is not easy to reproduce standalone!

Jarred-Sumner commented 5 months ago

@nathankleyn if you start the script with --conditions=worker set, does that fix it? my hunch is: 1) There is a bug in bun's node:http implementation where the agent in use by stripe is for http and not for https: https://github.com/stripe/stripe-node/blob/d14e271ff7ab285cc264bca1ea3781829266d5bb/src/net/NodeHttpClient.ts#L50 2) This bug did not manifest in Bun v1.0.xx because previously the "worker" ESM condition was included which caused "stripe" to load the fetch-based implementation (which does not have this bug) rather than the node:http implementation (which does have this bug) 3) This bug only manifests when an error occurs and the request is retried.

nathankleyn commented 5 months ago

Hey @Jarred-Sumner β€” thanks so much yet again for the super speed reply and debugging here! I can confirm adding --conditions=worker has resolved the issue on both 1.1.7+b0b7db5c0 and 1.1.8-canary.1+f52c17781.

Thank you so much for helping us get back up and running again β€” if there anything we can do to assist with the proper fix from here to the agent change, we're super happy to test or reproduce anything that you need. We'll also keep an eye on that Stripe PR you raised and see what happens once that gets merged too.

paulGeoghegan commented 3 months ago

@Jarred-Sumner it seems like the PR has been merged but I just ran in to this issue. It was definitely an issue with Bun as when I tried it with Node it worked fine. I'm using Bun 1.1.20.