harden fetch calls with timeouts and try/catch in actions/setup/js - #49670
Conversation
- add signal: AbortSignal.timeout(30_000) to apply_samples.cjs fetch (GitHub API) - add signal: AbortSignal.timeout(30_000) to artifact_client.cjs twirp fetch - wrap uploadFileToSignedURL fetch in try/catch + add 300s timeout - wrap listArtifacts fetch in try/catch + add 30s timeout - wrap downloadArtifact redirect fetch in try/catch + add 30s timeout - wrap downloadArtifact blob fetch in try/catch + add 300s timeout - add signal: AbortSignal.timeout(10_000) to check_version_updates.cjs fetch - add signal: AbortSignal.timeout(10_000) to send_otlp_span.cjs fetch Closes #49668 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
@copilot centralize timeout constants, no magic numbers |
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. Each file now declares a named constant — |
🔍 Triage Summary
Hardens fetch calls with timeouts/try-catch in actions/setup/js. Draft, no CI yet — part of eslint-monster cleanup wave. Automated triage — see the triage report for full context.
|
|
@copilot bump timeouts to at least 120 seconds |
There was a problem hiding this comment.
🟢 Ready to approve
The scoped changes consistently address the reported network-hardening gaps while preserving existing control flow.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Hardens setup-action network requests against hangs and transport failures.
Changes:
- Adds explicit 10–300 second fetch timeouts.
- Wraps artifact network failures with contextual errors.
- Preserves existing retry and graceful-failure behavior.
File summaries
| File | Description |
|---|---|
actions/setup/js/apply_samples.cjs |
Adds a timeout to PR metadata fetching. |
actions/setup/js/artifact_client.cjs |
Adds API/transfer timeouts and contextual fetch error handling. |
actions/setup/js/check_version_updates.cjs |
Adds a timeout to retried configuration fetching. |
actions/setup/js/send_otlp_span.cjs |
Adds a timeout to OTLP exports. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Balanced
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Done. All |
|
@copilot quick triage for this PR:
Run details: https://github.com/github/gh-aw/actions/runs/30743638240
|
Triage complete on current head
Local validation pass:
Remaining maintainer action: re-trigger CI on the latest head (agent pushes do not trigger workflows) and proceed once required checks are green. |
|
🎉 This pull request is included in a new release. Release: |
Several
fetch()calls inactions/setup/jswere missing abort signals (allowing requests to hang indefinitely) and some inartifact_client.cjswere unguarded againstTypeErroron network failures.Timeout signals added (
require-fetch-timeout)apply_samples.cjsartifact_client.cjs(Twirp RPC)artifact_client.cjs(list artifacts)artifact_client.cjs(redirect fetch)artifact_client.cjs(blob upload/download)check_version_updates.cjssend_otlp_span.cjstry/catch wrapping added (
require-fetch-try-catch)Four
await fetch(...)calls inartifact_client.cjswere not guarded against network-levelTypeError(DNS failures, connection refused, etc.). Each is now wrapped with a descriptive error re-throw:Affected call sites:
uploadFileToSignedURL,listArtifacts(loop body),downloadArtifact(redirect + blob).run: https://github.com/github/gh-aw/actions/runs/30743638240