From 2603f42b675007ee587fe72ff39e4458d50f77a3 Mon Sep 17 00:00:00 2001 From: PROTHAM <155388736+ProthamD@users.noreply.github.com> Date: Mon, 16 Mar 2026 23:16:08 +0530 Subject: [PATCH 1/5] fix: prevent Ctrl+C retries on web_fetch by distinguishing user abort from timeout --- packages/core/src/utils/fetch.test.ts | 31 ++++++++++++++++++++++++++- packages/core/src/utils/fetch.ts | 8 +++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/packages/core/src/utils/fetch.test.ts b/packages/core/src/utils/fetch.test.ts index 4ac0c7b3445..9d9985b18d5 100644 --- a/packages/core/src/utils/fetch.test.ts +++ b/packages/core/src/utils/fetch.test.ts @@ -100,7 +100,7 @@ describe('fetch utils', () => { }); describe('fetchWithTimeout', () => { - it('should handle timeouts', async () => { + it('should throw FetchError with ETIMEDOUT on an internal timeout', async () => { vi.mocked(global.fetch).mockImplementation( (_input, init) => new Promise((_resolve, reject) => { @@ -120,5 +120,34 @@ describe('fetch utils', () => { 'Request timed out after 50ms', ); }); + + it('should throw an AbortError (not ETIMEDOUT) when the caller signal is aborted', async () => { + vi.mocked(global.fetch).mockImplementation( + (_input, init) => + new Promise((_resolve, reject) => { + if (init?.signal) { + init.signal.addEventListener('abort', () => { + const error = new Error('The operation was aborted'); + error.name = 'AbortError'; + // @ts-expect-error - for mocking purposes + error.code = 'ABORT_ERR'; + reject(error); + }); + } + }), + ); + + const controller = new AbortController(); + // Abort the external signal before the request even starts + controller.abort(); + + const rejection = fetchWithTimeout('http://example.com', 10_000, { + signal: controller.signal, + }); + + await expect(rejection).rejects.toMatchObject({ name: 'AbortError' }); + // Must NOT be classified as a timeout + await expect(rejection).rejects.not.toThrow('timed out'); + }); }); }); diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index e339ea7fedd..d88af309159 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -150,6 +150,14 @@ export async function fetchWithTimeout( return response; } catch (error) { if (isNodeError(error) && error.code === 'ABORT_ERR') { + // If the caller's own signal was already aborted, this is a user-initiated + // cancellation (e.g. Ctrl+C), not an internal timeout. Re-throw as a plain + // AbortError so the retry layer does NOT treat it as a retryable ETIMEDOUT. + if (options?.signal?.aborted) { + const abortError = new Error('The operation was aborted.'); + abortError.name = 'AbortError'; + throw abortError; + } throw new FetchError(`Request timed out after ${timeout}ms`, 'ETIMEDOUT'); } throw new FetchError(getErrorMessage(error), undefined, { cause: error }); From 4250a3612d17505207fee2e121d59285abb21815 Mon Sep 17 00:00:00 2001 From: PROTHAM <155388736+ProthamD@users.noreply.github.com> Date: Fri, 27 Mar 2026 09:27:34 +0530 Subject: [PATCH 2/5] chore: save implementation plan for web-fetch abort fix --- implementation_plan.md | 248 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 248 insertions(+) create mode 100644 implementation_plan.md diff --git a/implementation_plan.md b/implementation_plan.md new file mode 100644 index 00000000000..f49d5ffab31 --- /dev/null +++ b/implementation_plan.md @@ -0,0 +1,248 @@ +# Test Output Silencing Implementation Plan + +## Goal +Reduce passing test output to single-line per test file so `npm run preflight` runs cleanly. + +## Phase 1: Core Silencing (Week 1-2) + +### 1.1 Vitest Configuration +- **Enable `silent: true`** in all vitest configs (packages/*/vitest.config.ts, evals/vitest.config.ts) +- **Current status**: core tests have `silent: true`; integration-tests and evals do not +- **Risk mitigation**: Logs only in errors; passing tests output nothing +- **Files to modify**: + - `integration-tests/vitest.config.ts` + - `evals/vitest.config.ts` + - `packages/sdk/vitest.config.ts` + - `packages/a2a-server/vitest.config.ts` + - `packages/test-utils/vitest.config.ts` + +### 1.2 Console Output Capture +Create centralized stdout/stderr suppression in test setups: +- **Location**: `packages/core/test-setup.ts` +- **Add**: + - `vi.spyOn(console, 'log').mockImplementation()` + - `vi.spyOn(console, 'error').mockImplementation()` + - `vi.spyOn(console, 'warn').mockImplementation()` + - `vi.spyOn(process.stdout, 'write').mockImplementation()` + - `vi.spyOn(process.stderr, 'write').mockImplementation()` +- **Teardown**: Reset mocks in afterEach + +### 1.3 Logger Mocking Framework +Create `packages/core/src/utils/loggerMocks.ts`: +- Mock `winston` logger (used for internal logging) +- Mock `chalk` for color stripping +- Mock `debug()` module +- Provide `unmockLogger()` for tests needing to debug logging +- Export as reusable in all test setups + +### 1.4 Expected Error Handling +- **Audit** all test files for unhandled error logs +- **Pattern**: Wrap expected errors in try-catch, don't let stack traces print +- **Script to identify**: Search for tests that call functions expected to throw without catching +- **Files affected**: ~80+ integration tests, ~200+ unit tests + +### 1.5 CLI Execution Output Buffering +Update `packages/test-utils/src/test-rig.ts`: +- **Buffer stdout/stderr** during `rig.run()` execution +- **Only print on test failure** (use Vitest lifecycle to detect) +- **Suppress intermediate logs** unless `VERBOSE=true` +- **Keep minimal pass/fail markers** for diagnostics + +--- + +## Phase 2: Audit & Fixes (Week 2-3) + +### 2.1 Test Output Audit +Run `npm run preflight` with logging enabled: +```bash +npm run test:ci 2>&1 | tee test-output.log +wc -l < test-output.log # Measure baseline +``` + +**By package**: +- `packages/core`: 20-30 files, ~100-200 tests +- `packages/sdk`: 10-15 files, ~50 tests +- `packages/a2a-server`: 5-10 files, ~30 tests +- `packages/cli`: 5-10 files, ~20 tests +- `integration-tests/`: 30-40 files, ~200-300 tests +- `evals/`: 15+ files (conditional, RUN_EVALS=1) + +### 2.2 Categorize Output Roots +- **Logger output**: Winston, debug module, custom loggers +- **CLI stderr**: gemini-cli running and printing errors +- **Third-party noise**: dependencies logging (axios, node, etc) +- **Legitimate failures**: Messages still needed for debugging + +### 2.3 Fix Test-by-Test +**Process**: +1. Run failing test batch with VERBOSE logging +2. Identify noise source +3. Apply targeted fix (mock, suppress, wrap in try-catch) +4. Verify no loss of legitimate error detection (run act() validation) +5. Repeat per test file + +**Priority order**: +1. Core package tests (foundation for others) +2. Integration tests (high noise volume) +3. SDK/CLI tests +4. Evals (lowest priority, conditional) + +--- + +## Phase 3: Automated Safeguards (Week 3-4) + +### 3.1 Vitest Reporter Plugin +**File**: `scripts/reporters/output-limit.reporter.ts` + +Implements custom reporter that: +- **Tracks per-test output** (stdout+stderr size) +- **Fails if > 5KB per test** (generous threshold for actual errors) +- **Reports summary**: + ``` + ✓ 1250 tests passed, 0 exceeded output limit + ⚠ 3 tests near limit (4.2KB, 3.8KB, 3.9KB) + ``` +- **Integration**: Add to all vitest configs as secondary reporter + +### 3.2 CI Output Validation +**GitHub Actions step** (`.github/workflows/test.yml`): +```yaml +- name: Check Test Output + if: success() # Only run if tests passed + run: | + npx vitest run --reporter=output-limit | grep -q "exceeded" && exit 1 + echo "✓ Test output within limits" +``` + +### 3.3 ESLint Rule: Unguarded Logger Detection +**File**: `eslint-rules/no-unguarded-logging.ts` + +Detects patterns: +- Direct `logger.error()` outside try-catch +- `console.log()` not wrapped in `if (VERBOSE)` +- `console.error()` in error handlers without suppression +- `throw new Error()` not caught in tests + +**Config**: Add to `.eslintrc.cjs`: +```js +'no-unguarded-logging': 'error' +``` + +### 3.4 Pre-commit Hook +**File**: `.husky/pre-commit` addition: +- Run `eslint --rule no-unguarded-logging` on changed test files +- Block commit if violations found +- Developers can override with `--no-verify` if intentional + +--- + +## Phase 4: Documentation & Maintenance (Week 4) + +### 4.1 Testing Guidelines +Update `CONTRIBUTING.md`: +- "Passing tests produce no output" +- Mock loggers in test setup +- Use `rig.run({ capture: true })` pattern +- Exception: use `--reporter=verbose` for debugging locally + +### 4.2 Logger Mock Library Docs +Create `docs/testing/logger-mocking.md`: +- How to unmock logger for debugging +- List of pre-configured mocks (winston, debug, etc) +- Examples for common logging libraries + +### 4.3 Regression Prevention +- **Monthly audit**: Re-run preflight and measure output +- **Alert threshold**: If output > 10x baseline, investigate +- **Dashboard**: Track test output trend in CI metrics + +--- + +## Implementation Details + +### A. Mock Order (Critical) +Must apply mocks in order: +1. **Vitest setup**: `silent: true`, spy setup +2. **Logger mocks**: Before any code runs +3. **Test execution** +4. **Teardown**: Reset all mocks + +### B. Preserving Legitimate Errors +**Not mocked by default**: +- Vitest assertion failures (these should print) +- Test timeout errors +- Unhandled promise rejections +- Syntax/runtime errors + +**Strategy**: Mock at app level, not Vitest level + +### C. Environment Flag for Debugging +Add `VERBOSE_TESTS=1` to: +- Disable console mocking +- Disable output buffering +- Show all logs, errors, timings + +Usage: +```bash +VERBOSE_TESTS=1 npm run test:scripts -- integration-tests/api-resilience.test.ts +``` + +### D. Phased Rollout +1. **Week 1**: Enable silencing in configs only (opt-in with VERBOSE flag) +2. **Week 2**: Fix individual tests with high noise +3. **Week 3**: Enforce with reporter plugin and lint rule +4. **Week 4**: Full integration in CI, update docs + +--- + +## Success Metrics + +| Metric | Target | Timeline | +|--------|--------|----------| +| Test files with <10 output lines | 100% | Week 2 | +| Output per test file | <1 line avg | Week 2 | +| lint rule violations | 0 | Week 3 | +| CI integration | Full | Week 4 | +| Developer adoption | 100% | Week 4 | + +--- + +## Risks & Mitigations + +| Risk | Mitigation | +|------|-----------| +| Silent tests hide real failures | Vitest reporter catches output anomalies; lint rule prevents unguarded errors | +| `act()` violations go undetected | Keep test timeout errors visible; React dev tools still catch in browser tests | +| Noise return over time | ESLint pre-commit hook + monthly audit | +| Developers bypass mocks | Document unmocking patterns; VERBOSE flag for debugging | + +--- + +## Quick Reference + +### Commands After Implementation +```bash +# Run preflight (should be quiet) +npm run preflight + +# Debug noisy tests with full logging +VERBOSE_TESTS=1 npm run test:ci + +# Run single test file verbosely +VERBOSE_TESTS=1 npm run test:scripts -- packages/core/src/__tests__/foo.test.ts + +# Check for policy violations ahead of commit +npm run lint:logging-policy +``` + +### File Checklist +- [ ] `packages/*/vitest.config.ts` - enable `silent: true` +- [ ] `packages/core/test-setup.ts` - add console mocks +- [ ] `packages/core/src/utils/loggerMocks.ts` - create mock library +- [ ] `packages/test-utils/src/test-rig.ts` - buffer output +- [ ] `scripts/reporters/output-limit.reporter.ts` - create reporter +- [ ] `eslint-rules/no-unguarded-logging.ts` - create rule +- [ ] `.eslintrc.cjs` - enable rule +- [ ] `.github/workflows/*.yml` - add CI checks +- [ ] `CONTRIBUTING.md` - update testing section +- [ ] `docs/testing/logger-mocking.md` - new guide From d412410088e698d95f1c13362370efda968f3de4 Mon Sep 17 00:00:00 2001 From: PROTHAM <155388736+ProthamD@users.noreply.github.com> Date: Tue, 31 Mar 2026 15:16:38 +0530 Subject: [PATCH 3/5] Update packages/core/src/utils/fetch.ts Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> --- packages/core/src/utils/fetch.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index 12514bcc2fd..81f3553f050 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -186,9 +186,7 @@ export async function fetchWithTimeout( // cancellation (e.g. Ctrl+C), not an internal timeout. Re-throw as a plain // AbortError so the retry layer does NOT treat it as a retryable ETIMEDOUT. if (options?.signal?.aborted) { - const abortError = new Error('The operation was aborted.'); - abortError.name = 'AbortError'; - throw abortError; + throw options.signal.reason ?? error; } throw new FetchError(`Request timed out after ${timeout}ms`, 'ETIMEDOUT'); } From 504c1ca42babf3e359b223ab1c675a7ac5e3d165 Mon Sep 17 00:00:00 2001 From: PROTHAM <155388736+ProthamD@users.noreply.github.com> Date: Sat, 9 May 2026 10:50:10 +0530 Subject: [PATCH 4/5] fix(core): address review comments on fetch abort handling --- .gitignore | 3 + implementation_plan.md | 248 -------------------------- packages/core/src/utils/fetch.test.ts | 23 ++- packages/core/src/utils/fetch.ts | 2 + 4 files changed, 22 insertions(+), 254 deletions(-) delete mode 100644 implementation_plan.md diff --git a/.gitignore b/.gitignore index ebb94151e8d..b259d2b9dbb 100644 --- a/.gitignore +++ b/.gitignore @@ -64,3 +64,6 @@ gemini-debug.log evals/logs/ temp_agents/ + +# Local implementation plan (not for submission) +implementation_plan.md diff --git a/implementation_plan.md b/implementation_plan.md deleted file mode 100644 index f49d5ffab31..00000000000 --- a/implementation_plan.md +++ /dev/null @@ -1,248 +0,0 @@ -# Test Output Silencing Implementation Plan - -## Goal -Reduce passing test output to single-line per test file so `npm run preflight` runs cleanly. - -## Phase 1: Core Silencing (Week 1-2) - -### 1.1 Vitest Configuration -- **Enable `silent: true`** in all vitest configs (packages/*/vitest.config.ts, evals/vitest.config.ts) -- **Current status**: core tests have `silent: true`; integration-tests and evals do not -- **Risk mitigation**: Logs only in errors; passing tests output nothing -- **Files to modify**: - - `integration-tests/vitest.config.ts` - - `evals/vitest.config.ts` - - `packages/sdk/vitest.config.ts` - - `packages/a2a-server/vitest.config.ts` - - `packages/test-utils/vitest.config.ts` - -### 1.2 Console Output Capture -Create centralized stdout/stderr suppression in test setups: -- **Location**: `packages/core/test-setup.ts` -- **Add**: - - `vi.spyOn(console, 'log').mockImplementation()` - - `vi.spyOn(console, 'error').mockImplementation()` - - `vi.spyOn(console, 'warn').mockImplementation()` - - `vi.spyOn(process.stdout, 'write').mockImplementation()` - - `vi.spyOn(process.stderr, 'write').mockImplementation()` -- **Teardown**: Reset mocks in afterEach - -### 1.3 Logger Mocking Framework -Create `packages/core/src/utils/loggerMocks.ts`: -- Mock `winston` logger (used for internal logging) -- Mock `chalk` for color stripping -- Mock `debug()` module -- Provide `unmockLogger()` for tests needing to debug logging -- Export as reusable in all test setups - -### 1.4 Expected Error Handling -- **Audit** all test files for unhandled error logs -- **Pattern**: Wrap expected errors in try-catch, don't let stack traces print -- **Script to identify**: Search for tests that call functions expected to throw without catching -- **Files affected**: ~80+ integration tests, ~200+ unit tests - -### 1.5 CLI Execution Output Buffering -Update `packages/test-utils/src/test-rig.ts`: -- **Buffer stdout/stderr** during `rig.run()` execution -- **Only print on test failure** (use Vitest lifecycle to detect) -- **Suppress intermediate logs** unless `VERBOSE=true` -- **Keep minimal pass/fail markers** for diagnostics - ---- - -## Phase 2: Audit & Fixes (Week 2-3) - -### 2.1 Test Output Audit -Run `npm run preflight` with logging enabled: -```bash -npm run test:ci 2>&1 | tee test-output.log -wc -l < test-output.log # Measure baseline -``` - -**By package**: -- `packages/core`: 20-30 files, ~100-200 tests -- `packages/sdk`: 10-15 files, ~50 tests -- `packages/a2a-server`: 5-10 files, ~30 tests -- `packages/cli`: 5-10 files, ~20 tests -- `integration-tests/`: 30-40 files, ~200-300 tests -- `evals/`: 15+ files (conditional, RUN_EVALS=1) - -### 2.2 Categorize Output Roots -- **Logger output**: Winston, debug module, custom loggers -- **CLI stderr**: gemini-cli running and printing errors -- **Third-party noise**: dependencies logging (axios, node, etc) -- **Legitimate failures**: Messages still needed for debugging - -### 2.3 Fix Test-by-Test -**Process**: -1. Run failing test batch with VERBOSE logging -2. Identify noise source -3. Apply targeted fix (mock, suppress, wrap in try-catch) -4. Verify no loss of legitimate error detection (run act() validation) -5. Repeat per test file - -**Priority order**: -1. Core package tests (foundation for others) -2. Integration tests (high noise volume) -3. SDK/CLI tests -4. Evals (lowest priority, conditional) - ---- - -## Phase 3: Automated Safeguards (Week 3-4) - -### 3.1 Vitest Reporter Plugin -**File**: `scripts/reporters/output-limit.reporter.ts` - -Implements custom reporter that: -- **Tracks per-test output** (stdout+stderr size) -- **Fails if > 5KB per test** (generous threshold for actual errors) -- **Reports summary**: - ``` - ✓ 1250 tests passed, 0 exceeded output limit - ⚠ 3 tests near limit (4.2KB, 3.8KB, 3.9KB) - ``` -- **Integration**: Add to all vitest configs as secondary reporter - -### 3.2 CI Output Validation -**GitHub Actions step** (`.github/workflows/test.yml`): -```yaml -- name: Check Test Output - if: success() # Only run if tests passed - run: | - npx vitest run --reporter=output-limit | grep -q "exceeded" && exit 1 - echo "✓ Test output within limits" -``` - -### 3.3 ESLint Rule: Unguarded Logger Detection -**File**: `eslint-rules/no-unguarded-logging.ts` - -Detects patterns: -- Direct `logger.error()` outside try-catch -- `console.log()` not wrapped in `if (VERBOSE)` -- `console.error()` in error handlers without suppression -- `throw new Error()` not caught in tests - -**Config**: Add to `.eslintrc.cjs`: -```js -'no-unguarded-logging': 'error' -``` - -### 3.4 Pre-commit Hook -**File**: `.husky/pre-commit` addition: -- Run `eslint --rule no-unguarded-logging` on changed test files -- Block commit if violations found -- Developers can override with `--no-verify` if intentional - ---- - -## Phase 4: Documentation & Maintenance (Week 4) - -### 4.1 Testing Guidelines -Update `CONTRIBUTING.md`: -- "Passing tests produce no output" -- Mock loggers in test setup -- Use `rig.run({ capture: true })` pattern -- Exception: use `--reporter=verbose` for debugging locally - -### 4.2 Logger Mock Library Docs -Create `docs/testing/logger-mocking.md`: -- How to unmock logger for debugging -- List of pre-configured mocks (winston, debug, etc) -- Examples for common logging libraries - -### 4.3 Regression Prevention -- **Monthly audit**: Re-run preflight and measure output -- **Alert threshold**: If output > 10x baseline, investigate -- **Dashboard**: Track test output trend in CI metrics - ---- - -## Implementation Details - -### A. Mock Order (Critical) -Must apply mocks in order: -1. **Vitest setup**: `silent: true`, spy setup -2. **Logger mocks**: Before any code runs -3. **Test execution** -4. **Teardown**: Reset all mocks - -### B. Preserving Legitimate Errors -**Not mocked by default**: -- Vitest assertion failures (these should print) -- Test timeout errors -- Unhandled promise rejections -- Syntax/runtime errors - -**Strategy**: Mock at app level, not Vitest level - -### C. Environment Flag for Debugging -Add `VERBOSE_TESTS=1` to: -- Disable console mocking -- Disable output buffering -- Show all logs, errors, timings - -Usage: -```bash -VERBOSE_TESTS=1 npm run test:scripts -- integration-tests/api-resilience.test.ts -``` - -### D. Phased Rollout -1. **Week 1**: Enable silencing in configs only (opt-in with VERBOSE flag) -2. **Week 2**: Fix individual tests with high noise -3. **Week 3**: Enforce with reporter plugin and lint rule -4. **Week 4**: Full integration in CI, update docs - ---- - -## Success Metrics - -| Metric | Target | Timeline | -|--------|--------|----------| -| Test files with <10 output lines | 100% | Week 2 | -| Output per test file | <1 line avg | Week 2 | -| lint rule violations | 0 | Week 3 | -| CI integration | Full | Week 4 | -| Developer adoption | 100% | Week 4 | - ---- - -## Risks & Mitigations - -| Risk | Mitigation | -|------|-----------| -| Silent tests hide real failures | Vitest reporter catches output anomalies; lint rule prevents unguarded errors | -| `act()` violations go undetected | Keep test timeout errors visible; React dev tools still catch in browser tests | -| Noise return over time | ESLint pre-commit hook + monthly audit | -| Developers bypass mocks | Document unmocking patterns; VERBOSE flag for debugging | - ---- - -## Quick Reference - -### Commands After Implementation -```bash -# Run preflight (should be quiet) -npm run preflight - -# Debug noisy tests with full logging -VERBOSE_TESTS=1 npm run test:ci - -# Run single test file verbosely -VERBOSE_TESTS=1 npm run test:scripts -- packages/core/src/__tests__/foo.test.ts - -# Check for policy violations ahead of commit -npm run lint:logging-policy -``` - -### File Checklist -- [ ] `packages/*/vitest.config.ts` - enable `silent: true` -- [ ] `packages/core/test-setup.ts` - add console mocks -- [ ] `packages/core/src/utils/loggerMocks.ts` - create mock library -- [ ] `packages/test-utils/src/test-rig.ts` - buffer output -- [ ] `scripts/reporters/output-limit.reporter.ts` - create reporter -- [ ] `eslint-rules/no-unguarded-logging.ts` - create rule -- [ ] `.eslintrc.cjs` - enable rule -- [ ] `.github/workflows/*.yml` - add CI checks -- [ ] `CONTRIBUTING.md` - update testing section -- [ ] `docs/testing/logger-mocking.md` - new guide diff --git a/packages/core/src/utils/fetch.test.ts b/packages/core/src/utils/fetch.test.ts index 911ca1dea88..dccac70c6d2 100644 --- a/packages/core/src/utils/fetch.test.ts +++ b/packages/core/src/utils/fetch.test.ts @@ -187,13 +187,24 @@ describe('fetch utils', () => { vi.mocked(global.fetch).mockImplementation( (_input, init) => new Promise((_resolve, reject) => { + const rejectWithAbortError = () => { + const error = new Error('The operation was aborted'); + error.name = 'AbortError'; + // @ts-expect-error - for mocking purposes + error.code = 'ABORT_ERR'; + reject(error); + }; + + // Handle the case where the signal is already aborted before + // fetch is called (e.g. controller.abort() called synchronously). + if (init?.signal?.aborted) { + rejectWithAbortError(); + return; + } + if (init?.signal) { - init.signal.addEventListener('abort', () => { - const error = new Error('The operation was aborted'); - error.name = 'AbortError'; - // @ts-expect-error - for mocking purposes - error.code = 'ABORT_ERR'; - reject(error); + init.signal.addEventListener('abort', rejectWithAbortError, { + once: true, }); } }), diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index 81f3553f050..d767fc0eeeb 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -186,6 +186,8 @@ export async function fetchWithTimeout( // cancellation (e.g. Ctrl+C), not an internal timeout. Re-throw as a plain // AbortError so the retry layer does NOT treat it as a retryable ETIMEDOUT. if (options?.signal?.aborted) { + // Rethrow the original abort reason or the caught error to preserve + // the stack trace and any custom abort reason (e.g. from Ctrl+C). throw options.signal.reason ?? error; } throw new FetchError(`Request timed out after ${timeout}ms`, 'ETIMEDOUT'); From 23a018eb68615b2e430658257a0f51f93ad3a9a9 Mon Sep 17 00:00:00 2001 From: PROTHAM <155388736+ProthamD@users.noreply.github.com> Date: Fri, 15 May 2026 12:06:37 +0530 Subject: [PATCH 5/5] fix(core): address PR review comments for web fetch utils --- packages/core/src/utils/fetch.test.ts | 14 +++++--------- packages/core/src/utils/fetch.ts | 4 ++-- 2 files changed, 7 insertions(+), 11 deletions(-) diff --git a/packages/core/src/utils/fetch.test.ts b/packages/core/src/utils/fetch.test.ts index 1f7ec1bd8b0..f14f75eea36 100644 --- a/packages/core/src/utils/fetch.test.ts +++ b/packages/core/src/utils/fetch.test.ts @@ -5,7 +5,7 @@ */ import { updateGlobalFetchTimeouts } from './fetch.js'; -import { describe, it, expect, vi, beforeEach, afterAll } from 'vitest'; +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import * as dnsPromises from 'node:dns/promises'; import type { LookupAddress, LookupAllOptions } from 'node:dns'; import ipaddr from 'ipaddr.js'; @@ -34,18 +34,14 @@ const { fetchWithTimeout, setGlobalProxy, } = await import('./fetch.js'); - -// Mock global fetch -const originalFetch = global.fetch; -global.fetch = vi.fn(); - interface ErrorWithCode extends Error { code?: string; } describe('fetch utils', () => { beforeEach(() => { - vi.clearAllMocks(); + // eslint-disable-next-line @typescript-eslint/no-explicit-any + vi.spyOn(global, 'fetch').mockImplementation(vi.fn() as any); // Default DNS lookup to return a public IP, or the IP itself if valid vi.mocked( dnsPromises.lookup as ( @@ -60,8 +56,8 @@ describe('fetch utils', () => { }); }); - afterAll(() => { - global.fetch = originalFetch; + afterEach(() => { + vi.restoreAllMocks(); }); describe('isAddressPrivate', () => { diff --git a/packages/core/src/utils/fetch.ts b/packages/core/src/utils/fetch.ts index 0b52535c2fc..ff22df0a34f 100644 --- a/packages/core/src/utils/fetch.ts +++ b/packages/core/src/utils/fetch.ts @@ -4,7 +4,7 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { getErrorMessage, isNodeError } from './errors.js'; +import { getErrorMessage, isAbortError } from './errors.js'; import { URL } from 'node:url'; import { Agent, ProxyAgent, setGlobalDispatcher } from 'undici'; import ipaddr from 'ipaddr.js'; @@ -202,7 +202,7 @@ export async function fetchWithTimeout( }); return response; } catch (error) { - if (isNodeError(error) && error.code === 'ABORT_ERR') { + if (isAbortError(error)) { // If the caller's own signal was already aborted, this is a user-initiated // cancellation (e.g. Ctrl+C), not an internal timeout. Re-throw as a plain // AbortError so the retry layer does NOT treat it as a retryable ETIMEDOUT.