Skip to content
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.

Sign up for GitHub

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

Jump to bottom

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

Open
nathankleyn opened this issue May 10, 2024 · 5 comments
Open

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

nathankleyn opened this issue May 10, 2024 · 5 comments
Assignees
@nektro
Labels
bug Something isn't working

Comments

@nathankleyn
Copy link

nathankleyn commented May 10, 2024 •
edited

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.
@nathankleyn nathankleyn added the bug Something isn't working label May 10, 2024
@nathankleyn nathankleyn changed the title Some Stripe API calls failing since uprading from Bun 1.0.x to Bun 1.1.x Some Stripe API calls failing in production since upgrading from Bun 1.0.x to Bun 1.1.x May 10, 2024
@Jarred-Sumner
Copy link
Collaborator

@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
Copy link
Author

@Jarred-Sumner Thanks for super fast reply! On it and will report back ASAP 👀 👍

@nathankleyn
Copy link
Author

@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
Copy link
Collaborator

Jarred-Sumner commented May 10, 2024 •
edited

@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.

Jarred-Sumner added a commit to Jarred-Sumner/stripe-node that referenced this issue May 10, 2024
@Jarred-Sumner
Use fetch-based client in Bun
f0e6601 
In Bun v1.0, for better overall ecosystem compatibility, we removed the `"worker"` package.json `"exports"` condition which unfortunately caused `"stripe"` to load the `"node:http"`-based implementation. This `"node:http"`-based implementation usually works fine, however there is an edgecase involving `https.Agent` having the wrong `protocol` set, (causing errors)[oven-sh/bun#10975] when a request is retried. This is a difficult to reproduce bug in Bun and not in `stripe`.

However, in Bun, `"node:http"` internally is implemented as a wrapper on top of `fetch`. So `"stripe"` might as well skip the wrapper and just use fetch instead, assuming that is not a breaking change in of itself.


If we add the `BUN_JSC_dumpModuleLoadingState=1` environment variable, Bun logs some information about which modules have loaded.

Bun v0.6.1:
```js
❯ BUN_JSC_dumpModuleLoadingState=1 bun-0.6.1 terminy.js 2>&1 | rg "\.esm.*.js"
Loader [resolve] ./node_modules/stripe/esm/stripe.esm.worker.js
Loader [fetch] ./node_modules/stripe/esm/stripe.esm.worker.js
loader [parsing] ./node_modules/stripe/esm/stripe.esm.worker.js
Loader [link] ./node_modules/stripe/esm/stripe.esm.worker.js
Loader [evaluate] ./node_modules/stripe/esm/stripe.esm.worker.js
```

Bun v1.1.7:
```js
❯ BUN_JSC_dumpModuleLoadingState=1 bun-1.1.7 terminy.js 2>&1 | rg "\.esm.*.js"
Loader [fetch] ./node_modules/stripe/esm/stripe.esm.node.js
loader [parsing] ./node_modules/stripe/esm/stripe.esm.node.js
Loader [link] ./node_modules/stripe/esm/stripe.esm.node.js
Loader [evaluate] ./node_modules/stripe/esm/stripe.esm.node.js
```
@Jarred-Sumner Jarred-Sumner mentioned this issue May 10, 2024
Open
@nathankleyn
Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@nektro nektro

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@nathankleyn @Jarred-Sumner @nektro