Skip to content

Remove native libcurl dependency and migrate to Node.js HTTP modules #131

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

Closed

Conversation

Jeongyong-park
Copy link
Contributor

Removed node-libcurl native library dependency and refactored upload logic to use Node.js built-in HTTP/HTTPS modules.

Changes
✅ Complete removal of node-libcurl native dependency
✅ Implemented file upload using Node.js built-in http/https modules with form-data
✅ Simplified installation and deployment process (no native compilation required)
✅ Improved cross-platform compatibility

Limitations

  • ⚠️ Loss of upload speed limiting functionality: Node.js built-in HTTP modules cannot implement curl's --upload-max-speed option
  • 📝 The upload-max-speed configuration option still exists but is no longer functional

Technical Details

  • Implemented FormData streaming in httpRequest function in libs/taskNew.js
  • Maintained parallel uploads (20 concurrent) and retry logic
  • Enhanced error handling and logging

Related Issues
Closes #130

ClusterODM/libs/taskNew.js

Lines 383 to 390 in 96b96ae

if (config.upload_max_speed) curl.setOpt(Curl.option.MAX_SEND_SPEED_LARGE, config.upload_max_speed);
// abort if slower than 30 bytes/sec during 1600 seconds */
curl.setOpt(Curl.option.LOW_SPEED_TIME, 1600);
curl.setOpt(Curl.option.LOW_SPEED_LIMIT, 30);
curl.setOpt(Curl.option.HTTPHEADER, [
'Content-Type: multipart/form-data'
]);

Signed-off-by: Jeongyong <polyline@kakao.com>
@Jeongyong-park
Copy link
Contributor Author

Speed limiting feature of curl is not implemented, but the it was works with Node.js 22 without relying on native libcurl.

@pierotofy
Copy link
Member

pierotofy commented Jun 9, 2025

Was AI used to create this pull request? It's OK if you did, just need to know.

@Jeongyong-park
Copy link
Contributor Author

Was AI used to create this pull request? It's OK if you did, just need to know.

you're right.

@pierotofy pierotofy requested a review from smathermather June 10, 2025 15:46
@smathermather
Copy link
Contributor

@Jeongyong-park -- for a sense, was this assisted by LLM / Co-pilot, or prompt-engineered (generated largely in whole by a prompt)?

What testing and review have you done of it? Thanks!

@Jeongyong-park
Copy link
Contributor Author

@Jeongyong-park -- for a sense, was this assisted by LLM / Co-pilot, or prompt-engineered (generated largely in whole by a prompt)?

What testing and review have you done of it? Thanks!

I used claude sonnet 4.0 model (LLM) in node 22 version and Cursor IDE

I already knew that there was a problem with running ClusterODM with node instead of docker in node 18 and node 20 version several years ago because of node-libcurl. and .nvmrc file alos points to version 15.10.0.

Regardless of the github issue, I personally tried to change node-libcurl related code to libraries such as axios or got several times, but it did not work well. After adding a debugging log and analyzing the log with LLM, I found that the cause was a problem with concurrency processing and asynchronous stability in the part of busboy processing files in libs/taskNew.js. I modified the code through LLM for this part.

I added nodeodm to 3001, 3002, 3003, etc. in docker locally for testing. After running clusterodm in the development environment 3000 times, I uploaded the banana images of odm-data in the webui and checked that the upload and transfer to nodeodm were processed normally and that the result file download was normal as before.

Regardless of whether this PR is merged or not, I think LLM is essential in the modern era for analyzing and modifying complex codes that do not have proper explanations in the typeless javascript language.

I think this commit is not implementing a new feature, but rather a maintenance that replaces a specific library that depends on the native library that was previously used with a node.js pure library that has similar functions.

Thank you.

@Jeongyong-park Jeongyong-park marked this pull request as draft June 11, 2025 07:07
@pierotofy
Copy link
Member

Given our recent discussions on AI usage, as well as the fact that we're losing some functionality, I will close this for now but will potentially refer to it in the future as a starting point for considering replacing libcurl. 🙏

@pierotofy pierotofy closed this Aug 19, 2025
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 this pull request may close these issues.

Consider replacing node-libcurl with modern HTTP clients to avoid native build issues
3 participants