Skip to content

Conversation

@RaMaTHA
Copy link

@RaMaTHA RaMaTHA commented Jan 28, 2026

Summary

This PR adds timeout handling for HTTP/HTTPS requests to prevent builds from hanging indefinitely when a remote service is unreachable or unresponsive.

Problem

While testing in an isolated environment, I noticed that HTTP requests made by the CLI do not have a timeout configured. If the target service cannot be reached, the request may wait indefinitely, causing the build to hang.

This became visible when running the CLI on a runner that is not capable of performing outbound HTTP requests. In this scenario, the build gets stuck waiting for a response that will never arrive.

Affected endpoint

One example where this occurs is the request to:

https://containers.dev/static/devcontainer-control-manifest.json

handled in: src/spec-configuration/controlManifest.ts

Solution

A timeout has been added to the HTTP request to ensure the process fails gracefully instead of hanging indefinitely when the service is unavailable.

@RaMaTHA RaMaTHA requested a review from a team as a code owner January 28, 2026 22:21
@abdurriq
Copy link
Contributor

Hello @RaMaTHA, thank you for your contribution! I agree that this is an annoying issue, but I worry that the approach in this PR will mask genuine errors or network issues, particularly because it does not throw an exception when failing. I also found that there are more idiomatic approaches now to cancelling node-http requests, such as the ones mentioned in this SO thread: https://stackoverflow.com/a/55021202

If you could implement the timeout mentioned in that thread (which applies to the connection part only), and throw an exception when timing out, I think this could be acceptable.

@RaMaTHA
Copy link
Author

RaMaTHA commented Jan 30, 2026

Hello @abdurriq, thank you for your fast response.

I have had a look at your suggestion regarding the timeout handling as mentioned in the SO thread you provided, and I agree that implementing a timeout for the connection part only is a better way of doing it. I have adjusted the code accordingly.

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.

3 participants