-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
Replace outdated http request dependency with Node 18's fetch
#7589
Comments
Thanks for opening this issue!
|
Do we even need to offer |
I would say there is no real reason why we should offer it. Perhaps we should remove it entirely from V5 as a breaking change? |
Maybe we could deprecate the method in v5 and remove it entirely in v6? If the method has security issues, then we could think about investing some time into using axios during the deprecation. |
httpRequest has issues. Sometimes request fails or constructed in worng way and server returns 4XX response. Axios works fine. I think we should remove it. |
If it is already severely broken in the way that you describe, where requests sometimes fail, we probably should make a small change to use axios, even while it's being deprecated. Because deprecation means it will still be available for a year. Not sure how broken it is though. |
On the back4app support group on slack someone faced this problem. Syntax was correct but server wasn't returning the response it should. İ suggested him to use node-fetch or axios to see if problem is under default httpRequest and he said problem is fixed when using axios. And like you said, one can easily add their preferred http library. Maintaining a default one should be avoided in my opinion. |
I see. I think we have 3 options:
Which would you prefer? |
I think option b makes sense. Option c is also possible but imo there is no point of fixing a bug of a deprecated feature. |
I would like to see it, and I think option b is the best, we could go with c but I think having direct aliases to other packages could result in developers assuming issues and documentation related with |
I think we agree on (b) then, sounds good to me. |
Closing via #7595 |
Reopened to discuss: @dblythy I just noticed that |
Parse.Cloud.httpRequest
Parse.Cloud.httpRequest
Parse.Cloud.httpRequest
I've renamed the title and objective of this issue to replace the outdated http request dependency with axios. You gave many good reasons why we should replace it. We have already deprecated it in #7595 and will remove its exposure in #8271. It seems that we still need to replace it, since it's also used internally. |
fetch
fetch
fetch
Node 18 provides a native method fetch that should make it unnecessary to add a dependency for http requests. Parse Server 7 will support Node >=18 (Node 16 EOL will be Sept 2023), and from then on we can switch to native More than 500 CI tests are currently using the request lib. Migrating to axios and possibly introducing bugs or flaky tests doesn't seem a necessary effort. Even though the request lib is outdated, the issues reported by developers were when using the From Parse Server 7 (2024) onwards, we can migrate to the native |
Maybe you can try I'd love to help you with this if you decide to give it a try. |
@ardatan I'm also linking the blog post you wrote about it: https://the-guild.dev/blog/fetch-for-servers |
I'm skeptical given the amount of changes we'd have to make throughout the repo and that only to bridge 2023. Unless there is another benefit? |
@mtrezza we are not sure it would be a big effort, are you open for us trying out and see if we can come up with a PR for it before Parse 6? |
New Feature / Enhancement Checklist
Current Limitation
Our docs state:
Why should we continue to support a legacy HTTP module, which hasn't been updated in 3 years, where there are more popular solutions that constantly update their security issues and vulnerabilities?
The http module also uses deprecated methods, such as
querystring
andparse
.Also
request
is archived, so we shouldn't recommend that either.We've seen Parse.Cloud.httpRequest be the source of many issues
Feature / Enhancement Description
Migrate to a dedicated HTTP library, such as axios
Example Use Case
Log
Parse.Cloud.httpRequest is depreciated. Please migrate to different HTTP request solution
Alternatives / Workarounds
axios
.3rd Party References
The text was updated successfully, but these errors were encountered: