Skip to content

Add timeout#1356

Open
dnsi0 wants to merge 2 commits intomainfrom
add-timeout
Open

Add timeout#1356
dnsi0 wants to merge 2 commits intomainfrom
add-timeout

Conversation

@dnsi0
Copy link
Copy Markdown
Contributor

@dnsi0 dnsi0 commented Apr 28, 2026

Fixes #1251 .

Changes proposed in this PR:

  • add timeout on docker pull image
  • fix detailed flag on status

@dnsi0
Copy link
Copy Markdown
Contributor Author

dnsi0 commented Apr 28, 2026

/run-security-scan

Copy link
Copy Markdown
Member

@alexcos20 alexcos20 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI automated code review (Gemini 3).

Overall risk: low

Summary:
This PR introduces a configurable download timeout for pulling Docker images in C2D jobs using AbortSignal.timeout(), effectively mitigating issues where jobs could hang indefinitely. It also introduces a minor cleanup in the status handler to exclude properties when C2D features are unavailable. The implementation is concise and well-designed.

Comments:
• [INFO][edge_case] Node's internal setTimeout (used by AbortSignal.timeout) accepts a maximum 32-bit signed integer value (2147483647 ms). If an operator misconfigures C2D_DOWNLOAD_TIMEOUT to a value greater than 2147483 seconds, the timeout will overflow and default to 1ms, resulting in immediate job failures. Consider clamping the maximum allowed value to prevent this edge case:

-      return parsed * 1000
+      return Math.min(parsed * 1000, 2147483647)

• [INFO][style] Great use of AbortSignal.timeout() here. It handles request/streaming timeouts at the core library level without the overhead of manual tracking and Promise.race() patterns. LGTM!

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.

C2D: Jobs stuck

3 participants