interledger / rafiki

An open-source, comprehensive Interledger service for wallet providers, enabling them to provide Interledger functionality to their users.
https://rafiki.dev/
Apache License 2.0
259 stars 89 forks source link

Replace axios #1036

Closed wilsonianb closed 1 month ago

wilsonianb commented 1 year ago
wilsonianb commented 1 year ago

global fetch is experimental so maybe we hold off? https://nodejs.org/en/blog/announcements/v18-release-announce/

mkurapov commented 8 months ago

Verify whether nock supports fetch before proceeding

BlairCurrey commented 8 months ago

Just wanted to note a potential gotcha if we switch over to fetch because this took me a while to figure out. The standard lib fetch does not automatically handle the Set-Cookie header as you might expect. We would need to parse this header and use them on subsequent requests ourselves. It may not be clear where exactly we will be receiving such a header so it may make sense to wrap fetch and do this in a central place (else we likely wont catch all cases) if we switch to fetch.

While doing the grant interaction flow (start, accept, finish), I noticed that the "finish" request did not find the koa session started by the initial "start" request. Digging a bit further, this is because the client did not send the session detail cookie on the finish request. The response from the start request contains these details in the Set-Cookie header but fetch doesn't do anything with it, regardless of the credentials option which usually controls it (https://developer.mozilla.org/en-US/docs/Web/API/fetch#credentials).

Thus when using fetch you need to parse the cookies from the Set-Cookie header and provide them on subsequent requests: https://github.com/interledger/rafiki/pull/2380/commits/cc939824bd12ba7cca1d27ecca9a0949e948a1e2#diff-87048db525e9563b074c43d7daba7be936621984eedcf27d2dd78c3fc1e25890R250-R267

edit: TIL this is simply how most http clients work outside the browser 😑 Was getting caught up by our axios client handling cookies in our consent screen but didn't stop to think that the difference is the browser not the client. Then realized axios didn't do this either. I see there are plugins like https://github.com/3846masa/axios-cookiejar-support that add this kind of support but I wonder why its not a more common feature. Perhaps because outside the browser you cant assume non-multi-tenancy?

mkurapov commented 6 months ago

If we end up working on this, I recommend using ky, since it would allow us to easily define a global http client with preset headers (with interceptors/hooks if we choose).

This was done in the open-payments repo already:

mkurapov commented 1 month ago

Closing - any new code changes will use native fetch (or ky, as mentioned above)