fal-ai / fal-js

The JavaScript client and utilities to fal-serverless with built-in TypeScript definitions
https://fal.ai
MIT License
91 stars 19 forks source link

feature request: configurations for replacing native fetch #70

Closed ZeroCho closed 3 months ago

ZeroCho commented 5 months ago

Currently, fal-js uses native fetch, but I think it is good to have configurations for changing a default network request function to axios or cross-fetch or got.

Also, I found out that cross-fetch is in package.json but it seems that it is not used in this package. I opened this issue, because I want to use cross-fetch instead of native fetch, because fetch has memory leak issue with datadog service that I'm using.

drochetti commented 5 months ago

Hey @ZeroCho this is a fair request, I'll look into adding a configurable fetch. We considered it initially, that's why cross-fetch is there, but the lack of control on how different consumers call the service can create a variety of errors we can't control or even easily debug, so that's why we currently lean against it.

I'm curious about the case you mentioned, does Datadog patch the global fetch?

ZeroCho commented 5 months ago

@drochetti Thank you for your reply. Yes, datadog patches global fetch and that causes a problem now. So it has a memory leak issue that even after an image generation is finished with fal, a fal agent keeps memory of images. I found out that it was not the problem of fal, but the problem of fetch that datadog patched. That's why I wanna change fetch function.

https://github.com/DataDog/dd-trace-js/issues/4286

drochetti commented 3 months ago

Hey @ZeroCho this feature was added and already released. See the PR here: https://github.com/fal-ai/fal-js/pull/74

I'll close this isse, feel free to reach out if you have any other issues or feedback.

ZeroCho commented 3 months ago

Thank you! It works like a charm!