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.

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

Importing Stripe via ES Modules does not respect monkey-patching 'http' #1844

Closed
janhesters opened this issue Jul 14, 2023 · 12 comments · Fixed by #1854
Closed

Importing Stripe via ES Modules does not respect monkey-patching 'http' #1844

janhesters opened this issue Jul 14, 2023 · 12 comments · Fixed by #1854
Assignees
Labels

Comments

@janhesters
Copy link

janhesters commented Jul 14, 2023

Describe the bug

When trying to mock responses made with stripe-node in a Node.js enviroment, the requests hit the real server instead of getting caught by MSW.

To Reproduce

You can copy and paste the code below. (Either write actual handlers for your stripeHandlers, or you can leave them out, as we did below. If you want to try it out with handlers, simply spread them into setupServer.)

Sine we configured onUnhandledRequest: 'warn' MSW should log out to the console when requests are uncaught. The manual method with axios works correctly and logs out this warning / reacts when you configure handlers.

However, the second (in the code blocked skipped) test does NOT log out this warning and simply hits the servers.

// @vitest-environment node
import axios from 'axios';
import { rest } from 'msw';
import { setupServer } from 'msw/node';
import Stripe from 'stripe';
import invariant from 'tiny-invariant';

import { stripeHandlers } from '~/test/mocks/handlers/stripe';

invariant(
  process.env.STRIPE_SECRET_KEY,
  'STRIPE_SECRET_KEY environment variable is required',
);

const stripeAdmin = new Stripe(process.env.STRIPE_SECRET_KEY, {
  apiVersion: '2022-11-15',
});

const getProducts = async () => {
  try {
    const response = await axios.get('https://api.stripe.com/v1/products', {
      headers: {
        Authorization: `Bearer ${process.env.STRIPE_SECRET_KEY}`,
      },
      params: { active: true },
    });

    console.log(response.data);
  } catch (error) {
    console.error(error);
  }
};

const server = setupServer();

beforeAll(() => server.listen({ onUnhandledRequest: 'warn' }));
afterEach(() => server.resetHandlers());
afterAll(() => server.close());

describe('getProducts()', () => {
  test('it works', async () => {
    const actual = await getProducts();
    const expected: any = [];

    expect(actual).toEqual(expected);
  });
});

describe.skip('stripeAdmin', () => {
  test('getting products works', async () => {
    const actual = await stripeAdmin.products.list();
    const expected: any = [];

    expect(actual).toEqual(expected);
  });
});
import { rest } from 'msw';

import { mockStripePrices, mockStripeProducts } from '../fixtures/stripe';

export const stripeHandlers = [
  rest.get('https://api.stripe.com/v1/products', (request, response, context) =>
    response(context.json(mockStripeProducts)),
  ),
  rest.get('https://api.stripe.com/v1/prices', (request, response, context) =>
    response(context.json(mockStripePrices)),
  ),
];

Expected behavior

MSW can intercept the Stripe requests.

I assume this library does HTTP requests somehow esoterically, so it would be nice to learn how it does it, so that we can get MSW to work.

Code snippets

No response

OS

macOS

Node version

18.6.0

Library version

stripe-node v12.12.0

API version

2022-11-15

Additional context

No response

@janhesters janhesters added the bug label Jul 14, 2023
@richardm-stripe
Copy link
Contributor

Hello @janhesters, thank you for writing in.

stripe-node by default uses the https.request from the Node.js standard library.

I don't know a whole lot about the MSW library, but their documentation says

Since Service Worker API cannot run in a non-browser environment, the NodeJS support of request interception is done via a designated node-request-interceptor library.

That link took me to https://github.com/mswjs/interceptors#interceptors, and so based on that I made a little test to see if the underlying "ClientRequestInterceptor" is capable of intercepting requests from stripe-node.

// server.js
const { ClientRequestInterceptor } = require('@mswjs/interceptors/ClientRequest')
const interceptor = new ClientRequestInterceptor()
interceptor.apply()
interceptor.on('request', ({ request, requestId }) => {
  console.log('INTERCEPTED: ', request.method, request.url)
  request.respondWith(
      new Response(
          JSON.stringify({data: [{"foo": "bar"}]}),
      )
    )
  })

const stripe = require("stripe")("sk_test_xyz")
stripe.customers.list().then(lst => {
  console.log('RECEIVED', lst)
});

Outputs:

$ node server.js
INTERCEPTED:  GET https://api.stripe.com/v1/customers
RECEIVED { data: [ { foo: 'bar' } ] }

This would seem to absolve stripe-node of sending requests in an uninterceptable way. Maybe there's a different way to configure your test project that can get interception to succeed, or a problem in MSW itself?

@richardm-stripe richardm-stripe self-assigned this Jul 15, 2023
@janhesters
Copy link
Author

@richardm-stripe Interesting!

Thank you so much for your fast reply, I'll look into it some more.

@janhesters
Copy link
Author

@richardm-stripe Interestingly, this seems to be tied to the usage of the new keyword as well as Promise.all.

Consider the code snippet below:

// @vitest-environment node
import { setupServer } from 'msw/node';
import Stripe from 'stripe';
import invariant from 'tiny-invariant';
import { describe, expect, test } from 'vitest';

import {
  mockStripePrices,
  mockStripeProducts,
} from '~/test/mocks/fixtures/stripe';
import { stripeHandlers } from '~/test/mocks/handlers/stripe';


invariant(
  process.env.STRIPE_SECRET_KEY,
  'STRIPE_SECRET_KEY environment variable is required',
);

const server = setupServer(...stripeHandlers);

beforeAll(() => server.listen({ onUnhandledRequest: 'warn' }));
afterEach(() => server.resetHandlers());
afterAll(() => server.close());

describe('with require', () => {
  const stripe: Stripe = require('stripe')(process.env.STRIPE_SECRET_KEY);

  test('it works', async () => {
    const products = await stripe.products.list();

    const actual = products.data;
    const expected = mockStripeProducts;

    expect(actual).toEqual(expected);
  });

  describe('with sequential fetches', () => {
    test('it works', async () => {
      async function fetchStripeProductsWithPrices() {
        const products = await stripe.products.list();
        const prices = await stripe.prices.list();

        return { products: products.data, prices: prices.data };
      }

      const actual = await fetchStripeProductsWithPrices();
      const expected = {
        products: mockStripeProducts,
        prices: mockStripePrices,
      };

      expect(actual).toEqual(expected);
    });
  });


  describe('with parallel fetches', () => {
    test.skip('it times out', async () => {
      async function fetchStripeProductsWithPrices() {
        const [products, prices] = await Promise.all([
          stripe.products.list(),
          stripe.prices.list(),
        ]);

        return { products: products.data, prices: prices.data };
      }

      const actual = await fetchStripeProductsWithPrices();
      const expected = {
        products: mockStripeProducts,
        prices: mockStripePrices,
      };

      expect(actual).toEqual(expected);
    });
  });
});

describe.skip('with new keyword', () => {
  test('it fails', async () => {
    const stripe = new Stripe(process.env.STRIPE_SECRET_KEY, {
      apiVersion: '2022-11-15',
    });
    const products = await stripe.products.list();

    const actual = products.data;
    const expected = mockStripeProducts;

    expect(actual).toEqual(expected);
  });
});

A few things to note:

  1. Employing require and opting for sequential usage of the Stripe's Node SDK is successful.
  2. If require is used followed by the SDK for parallel requests, the requests time out because the responses aren't delivered, even though the handlers receive the request.
  3. Using new is never successful.
  4. You need to skip either or the other approach because when you don't skip and use both - require in one describe and import with new in the other describe - the new and import approach suddenly starts working. We speculate that this is due to certain side effects or mutable states related to how Node.js imports things.

We're going to go with the const stripe: Stripe = require('stripe')(process.env.STRIPE_SECRET_KEY); for our prod code now, as its good enough for us, unblocks us, and I have no idea how to debug this to get to the root cause.

We do suspect either in Stripe or in MSW there is something wrong.

And our guess is with the Stripe node SDK, because all other SDK's that we're using (e.g OpenAI, Sendgrid, Twitter, etc.) all work fine no matter how we use them. So we suspect there is something special with the Stripe node SDK in terms of how it sends requests.

Even when we wrote manual calls with Axios to the Stripe API, they worked.

Thank you again, @richardm-stripe, for your help! 🙏

@richardm-stripe
Copy link
Contributor

I think I tracked down what's happening here. See mswjs/interceptors#201

MSW won't work if you do

import * as https from 'https';

only

import https from 'https';

And in stripe-node, we do the former. I'll briefly investigate to confirm switching is feasible.

@richardm-stripe
Copy link
Contributor

I found nock/nock#2461 too.

@janhesters
Copy link
Author

@richardm-stripe Wow, thank you for digging! That looks like a promising lead.

@richardm-stripe
Copy link
Contributor

This should be fixed in the next release on Thursday, leaving it open until then.

@janhesters
Copy link
Author

@richardm-stripe Thanks a ton! 🙏

@richardm-stripe
Copy link
Contributor

Turns out #1854 is not viable because esModuleInterop: true is a breaking change and not recommended for libraries to enable.

I think we can still find a way to make a similar idea work but we won't land it today as expected.

@janhesters
Copy link
Author

@richardm-stripe Got it, thank you for the update!

@richardm-stripe richardm-stripe changed the title MSW doesn't catch requests made with stripe-node node 'http' cannot be monkey-patched if Stripe is imported via ES Modules Aug 2, 2023
@richardm-stripe richardm-stripe changed the title node 'http' cannot be monkey-patched if Stripe is imported via ES Modules Importing Stripe via ES Modules does not respect monkey-patching 'http' Aug 2, 2023
@anniel-stripe
Copy link
Contributor

Hi! Apologies for the wait here. We took another stab at fixing this in #1866, which has been included in today's release.

I'll close out this issue as 'http' / 'https' modules should be monkey-patchable starting from v12.17.0. Thanks again for raising this issue!

@janhesters
Copy link
Author

@anniel-stripe amazing, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants