Use async functions within request handling code

[rtpg]

Is your feature request related to a problem? Please describe.

Currently, there is a lot of setTimeouts and other non-async-y code that means that we cannot take advantage of V8's async stack trace feature. This means that we lose out on stack traces, the bare minimum that one would want for debugging an issue in production.

While support for the async stack trace feature came in Node 12, support for the async/await syntax (which would allow for users of newer node to get a stack trace) has been present from Node 7.

Describe the solution you'd like

At the very least, please use async/await syntax within the core request calling methods (StripeResource, StripeMethod), and remove usage of raw setTimeout, instead doing things like await Promise((resolve, reject) => setTimeout(reject, 0)) to make sure that async stack traces are maintained.

Doing this pervasively should make the code easier to manage, reduce a lot of indentation, and be functionally equivalent given how this all works.

Describe alternatives you've considered

Monkeypatching is theoretically possible, and there are options to capture error traces myself, but that involves creating spurious error objects all over and increasing difficulties of updating stripe-node.

Additional context

#1066 is an example of this issue

[rtpg]

I'm actually a bit convinced the stack trace breakage was at one point being done intentionally from this utility function, at least for some methods.

While if you do things in the most straightforward way, retries will lead to deeper callstacks, there is another option that is also available and easy to express with async/await: an "iterative" for loop. It's understandably harder to express with promises since you can only go deeper in a callstack with promises.

I might be totally offbase on the purpose of that utility function or even on how much using the async keyword will fix my problem, though. But I would appreciate somebody responsible for this library to look at it.

[yejia-stripe]

Hi @rtpg! Thank you for the report! We'll look into this and circle back when we have more information. In the meantime, does the workaround from #1066 work for you?

[rtpg]

@yejia-stripe sorry for taking a while to get back. Unfortunate the workaround does not work for usages around async iteration:

for await (const sub of wrappedStripe.subscriptions.list({limit: 10})) {
    console.log(sub)

^ this code snippet doesn't work with the proxying (I imagine because the list implements both async iteration and a standard promise mechanism).

[rtpg]

After working on this more, and trying to get the proxying to work, I found that using the proxy means that autoPagingEach and friends will also not work. I would really like to not have to choose between "getting autopaging" and "getting stack traces".

To be honest this is a really frustrating issue, doubly so because the cleanup job is straightforward, and because full stack traces are such a basic expectation for debugging issues. I understand everyone has their priorities but please consider this a very enthusiastic whine about this library's usability.

[ramya-stripe]

Thanks for getting back to us @rtpg
We will take another look next week

[ramya-stripe]

Hey @rtpg

Thanks for your patience.
We agree that this is crucial for the debugging experience.
This would be a bit more than a quick fix and so we will be working on this in the coming months.

[rtpg]

thank you for the comment! I'm glad to hear it, and I think that it will make many people happy