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

Should drop the polyfills in favor of native fetch or a newer library #169

Open
thgreasi opened this issue Nov 1, 2022 · 3 comments · May be fixed by #185
Open

Should drop the polyfills in favor of native fetch or a newer library #169

thgreasi opened this issue Nov 1, 2022 · 3 comments · May be fixed by #185

Comments

@thgreasi
Copy link
Contributor

thgreasi commented Nov 1, 2022

Node 18 now has native fetch and webstreams are available since node 16.
On the other hand we might want to consider moving to a different more feature rich library like got.
See: https://balena.zulipchat.com/#narrow/stream/346007-balena-io.2FbalenaCloud/topic/What.20we.20replace.20'request'.20with.3F

@Benedict-Carling
Copy link

Benedict-Carling commented Feb 3, 2025

Hi @thgreasi, @balena-ci

I've encountered severe memory issues with fetch-ponyfill in our Node.js server that have caused multiple memory-related restarts.

Important

Please let me know if you intend on continuing with your PR #185 , it looks close and it would be very helpful

I would find if very helpful to use the native fetch version as it would allow me to use node-fetch v3 when operating in Node 20 or node 22, v3 has improved streaming handling capabilities which I believe would reduce the memory related issues I am seeing.

As fetch-ponyfill uses node-fetch ~2.6.1 regardless of which node environment you are operating on, by using it I am unable to gain the benefits of native node fetch in node 20 or 22

{"callFrame":{
  "functionName":"",
  "url":"/app/node_modules/fetch-ponyfill/node_modules/node-fetch/lib/index.js",
  "lineNumber":269
},
"selfSize":8389272}

Please let me know if you would like more details

@thgreasi
Copy link
Contributor Author

thgreasi commented Feb 14, 2025

Hi,
@Benedict-Carling that's the first time I hear about balena-request causing memory issues, so thanks for raising it.
I do want to get #185 to the finish line for sure. Code wise & as far I was able to test it with the balena-sdk & balena-cli it seems fine, but for some strange reason I'm facing issues with the tests failing. More interestingly running the tests in isolation works, but running them all together is when it fails, and at the same time the were passing for node 18 just fine.
I can't dedicate time on this right now, but I will raise this internally.

@thgreasi thgreasi linked a pull request Feb 14, 2025 that will close this issue
@Benedict-Carling
Copy link

@thgreasi Thank you for your response, I'm happy to have a look at the tests myself when I get time

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

Successfully merging a pull request may close this issue.

2 participants