refactor: migrate axios to native fetch by tony8713 · Pull Request #457 · snapshot-labs/stamp

tony8713 · 2026-06-09T02:37:33Z

What

Migrates the HTTP client from axios to node-fetch, as requested in #206.

node-fetch is already a dependency and is used across several resolvers, so this consolidates onto a single HTTP client and drops axios (and its now-orphaned transitive follow-redirects).

Why now

Issue #206 (refactor: migrate axios to node-fetch) was effectively blocked by the old prod Node version (Node 16), where axios was kept around. With prod's build/EAS Node recently bumped (18 -> 20), the modern fetch stack is available and axios is no longer needed, so the migration can land.

Changes

src/utils.ts - graphQlCall now uses node-fetch. It preserves the previous axios { data: <body> } response shape so every caller that destructures { data: { data } } keeps working unchanged. Non-2xx responses throw a FetchError carrying response.status, so the existing 504 silencing in isSilencedError still triggers.
src/resolvers/utils.ts - replaces axiosDefaultParams with keep-alive http/https agents wired through node-fetch (fetchWithKeepAlive, defaultFetchParams, DEFAULT_TIMEOUT). fetchHttpImage now uses response.buffer().
src/resolvers/starknet.ts and src/addressResolvers/starknet.ts - ported to node-fetch. Non-ok responses throw so Promise.allSettled keeps the same semantics axios provided (rejection on non-2xx).
Removes axios and orphaned follow-redirects from package.json and yarn.lock.

Verification

yarn typecheck clean
yarn lint clean
addressResolvers/utils unit tests pass (incl. the axios-shape 504 silencing tests, which still pass against the new FetchError shape)

Closes #206

codecov · 2026-06-09T09:55:24Z

Codecov Report

❌ Patch coverage is 95.34884% with 2 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
src/utils.ts	91.66%	2 Missing ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Replaces the axios HTTP client with node-fetch across the resolvers and address resolvers, dropping the extra dependency now that prod runs Node 20 (native/modern fetch stack, axios no longer needed for Node 16 support). - src/utils.ts: graphQlCall uses node-fetch and preserves the previous `{ data: <body> }` response shape so callers stay unchanged; throws a FetchError carrying `response.status` on non-2xx to keep the existing 504 silencing behavior in isSilencedError. - src/resolvers/utils.ts: replaces axiosDefaultParams with keep-alive http/https agents wired through node-fetch (fetchWithKeepAlive, defaultFetchParams); fetchHttpImage now uses response.buffer(). - src/resolvers/starknet.ts + src/addressResolvers/starknet.ts: ported to node-fetch; non-ok responses throw so Promise.allSettled keeps the same semantics axios had (rejection on non-2xx). - Removes axios (and now-orphaned follow-redirects) from package.json and yarn.lock. Closes #206 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Node >=22 ships a global fetch (undici), so the node-fetch dependency is unnecessary. Replace all node-fetch imports with the native global fetch: - Drop the http/https keep-alive agents in resolvers/utils.ts (undici pools connections by default) and switch the request timeout to AbortSignal.timeout. - Replace node-fetch-only options (agent, timeout) with native equivalents. - Use Buffer.from(await response.arrayBuffer()) in place of response.buffer(). - Remove node-fetch from dependencies. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Native fetch (undici) keeps connections alive by default, so the fetchWithKeepAlive wrapper only injected a default timeout. Inline that timeout at the call sites and remove the passthrough wrapper. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

… migration Mocks global.fetch (no network, unlike the live integration/e2e suites) to lock in the axios -> native fetch behaviour of graphQlCall: AbortSignal.timeout wiring, the preserved `{ data: <body> }` response shape, header filtering, and the non-2xx FetchError whose response.status keeps isSilencedError's 504 silencing working after the wrapper removal. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Per review feedback, removing the fetch-mocked unit test added earlier; it was exploratory. Native fetch migration and wrapper removal stay. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot

Pull request overview

This PR removes axios usage and refactors HTTP calls across utilities/resolvers to use the Fetch API with AbortSignal.timeout, while cleaning up dependencies/lockfile accordingly.

Changes:

Replace axios-based GraphQL and resolver HTTP calls with fetch + timeout signals.
Remove axios (and follow-redirects) from dependencies and update yarn.lock.
Remove node-fetch imports and the direct node-fetch dependency, relying on global fetch.

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
yarn.lock	Removes `axios` / `follow-redirects` entries and updates `node-fetch` resolution keys.
package.json	Drops `axios` and direct `node-fetch` dependency declarations.
src/utils.ts	Refactors `graphQlCall` to use `fetch` and introduces a local `FetchError` wrapper.
src/resolvers/utils.ts	Replaces axios image fetching helper with a `fetch`-based implementation and shared timeout constant.
src/resolvers/starknet.ts	Ports Starknet resolver HTTP fetching to `fetch` + timeout signal.
src/resolvers/farcaster.ts	Removes `node-fetch` import in favor of global `fetch`.
src/lookupDomains/shibarium.ts	Removes `node-fetch` import in favor of global `fetch`.
src/addressResolvers/starknet.ts	Ports Starknet address resolver bulk API calls from axios to `fetch`.

Comments suppressed due to low confidence (1)

package.json:46

The PR title/description say the codebase is consolidating onto node-fetch, but this change removes the node-fetch dependency and the code now uses the global fetch instead. Please either (a) update the PR title/description to reflect a migration to the built-in fetch/undici stack, or (b) keep node-fetch as a direct dependency and import it where needed.

    "@snapshot-labs/snapshot.js": "^0.14.21",
    "@unstoppabledomains/resolution": "^9.2.2",
    "@webinterop/dns-connect": "^0.3.1",
    "compression": "^1.7.4",
    "cors": "^2.8.5",
    "dotenv": "^16.0.0",
    "express": "^4.17.1",
    "jsdom": "^19.0.0",
    "nodemon": "^2.0.7",
    "redis": "^4.6.10",
    "sharp": "^0.34.5",
    "starknet": "^6.11.0",
    "ts-node": "^10.8.1",
    "typescript": "^4.7.3"

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

 export async function fetchHttpImage(url: string): Promise<Buffer> {
-  return (
-    await axios({
-      url,
-      ...{
-        responseType: 'arraybuffer',
-        ...axiosDefaultParams
-      }
-    })
-  ).data;
+  const response = await fetch(url, { signal: AbortSignal.timeout(DEFAULT_TIMEOUT) });
+  return Buffer.from(await response.arrayBuffer());
 }


 async function fetchImageOrMetadata(url: string): Promise<Buffer | { image?: string }> {
-  const response = await axios({
-    url,
-    responseType: 'arraybuffer',
-    ...axiosDefaultParams
-  });
-  const contentType: string = response.headers['content-type'] || '';
-  const data = Buffer.from(response.data);
+  const response = await fetch(url, { signal: AbortSignal.timeout(DEFAULT_TIMEOUT) });
+  const contentType: string = response.headers.get('content-type') || '';
+  const data = Buffer.from(await response.arrayBuffer());


+    if (!response.ok) {
+      throw new Error(`Starknet API request failed with status ${response.status}`);
+    }
+
+    return response.json() as Promise<Record<string, string>>;


+  if (!response.ok) {
+    throw new FetchError(`GraphQL request failed with status ${response.status}`, response.status);
+  }
+
+  // Preserve the previous axios response shape (`{ data: <body> }`) so callers
+  // that destructure `{ data: { data } }` keep working.
+  return { data: await response.json() };
+}
+
+class FetchError extends Error {
+  response: { status: number };
+
+  constructor(message: string, status: number) {
+    super(message);
+    this.name = 'FetchError';
+    this.response = { status };
+  }
 }


wa0x6e

Review: axios → native fetch migration

Happy path is clean (timeouts ported to AbortSignal.timeout, graphQlCall return shape preserved, deps removed consistently). The risk is error semantics: axios threw on any non-2xx and carried .code/message error shapes; native fetch does neither. The code that depended on both wasn't fully ported.

🔴 Blocker

src/resolvers/utils.ts — fetchHttpImage no longer checks response.ok. Native fetch resolves on 4xx/5xx, so a 404/500 error body is read as image bytes. On cover/logo paths the buffer returns unvalidated (src/api.ts resize) outside the resolver fallback, so an expired/rate-limited image URL can bypass fallback and 500 the request. Restore axios semantics: check response.ok, throw with status/url on non-2xx. Add a test for space-cover/user-cover where the upstream returns 404 → assert fallback, not a sharp error.

🟡 Major

src/resolvers/starknet.ts — fetchImageOrMetadata no longer checks response.ok. Same class of bug: a non-2xx body is JSON-parsed or fed as image bytes into resize()/sharp instead of failing cleanly.
src/addressResolvers/utils.ts — isSilencedError no longer matches native-fetch timeouts. AbortSignal.timeout() rejects with a DOMException name: 'TimeoutError', no .code; the includes('TIMEOUT') check is case-sensitive and won't match. axios timeouts carried ECONNABORTED/ETIMEDOUT which did match. Routine upstream timeouts now flood Sentry.
src/addressResolvers/utils.ts:64 — isSilencedError never inspects error.cause, so socket-level errors escape. undici surfaces network failures (ECONNRESET, ETIMEDOUT, ENOTFOUND, ECONNREFUSED) as a TypeError with message: 'fetch failed' and the real errno on error.cause.code. The codes array reads error.code / error.response?.status / error.error?.code but not error.cause?.code, and the message is 'fetch failed'. So ECONNRESET/ETIMEDOUT — which are in the silenced list — no longer match: the migration moved them from error.code to error.cause.code. Fix: add error.cause?.code to the codes array.
src/addressResolvers/lens.ts — MUTED_ERRORS no longer matches. The new FetchError message is GraphQL request failed with status 503; the old substrings (status code 503/429) don't appear, and 503/429 aren't in the silenced-codes list. Lens rate-limit/outage now floods Sentry.
src/lookupDomains/ens.ts — Sentry context reads err.response.data, now always undefined. The new FetchError sets only .response.status. Diagnostic regression (optional chaining avoids a crash, but the upstream error body is lost in reports).

🔵 Minor / ⚪ Nit

src/utils.ts — new FetchError(message, status) collides by name with the empty marker FetchError in src/addressResolvers/utils.ts. Different shapes, same name → instanceof/import hazard. Consider GraphQlError.
src/utils.ts — FetchError.response.status is set at the throw site but read by no caller. Dead state.
5e3 timeout hardcoded in src/utils.ts + src/addressResolvers/starknet.ts while src/resolvers/utils.ts exports DEFAULT_TIMEOUT for the same value — three expressions of one timeout.
src/resolvers/utils.ts — removed the keepAlive http/https Agents; pooling now relies on undici defaults. Possible handshake overhead under high-volume image resolution.
src/utils.ts — method: 'post' → convention is 'POST' (fetch normalizes it; harmless).

Bottom line: the blocker plus the three silencing/diagnostic findings are the same root cause — a parity pass is needed over every site that branched on axios error shape (response.ok gating + .code/message-keyed silencing). Findings 1–3 worth covering with tests since they're silent regressions.

Address review feedback on #457. Native fetch resolves on non-2xx and does not carry axios-style error shapes, so several sites silently regressed: - resolvers/utils.ts fetchHttpImage + resolvers/starknet.ts fetchImageOrMetadata: check response.ok and throw on non-2xx, so error bodies are no longer fed to sharp/JSON.parse (cover/logo paths now fall back instead of 500-ing). - utils.ts graphQlCall: rename FetchError -> GraphqlError (avoids the name collision with the marker class in addressResolvers/utils), put the status in the message ("status code <n>") and capture the response body on error.response.data so isSilencedError matching and Sentry context capture keep working. - addressResolvers/utils.ts isSilencedError: also inspect error.cause?.code (undici socket errors) and error.name (AbortSignal.timeout -> TimeoutError), match codes case-insensitively, and add ENOTFOUND/ECONNREFUSED. - addressResolvers/starknet.ts: throw an error carrying response.status so transient upstream statuses (e.g. 504) stay silenced; dedupe the timeout via the shared DEFAULT_TIMEOUT. - utils.ts: hoist DEFAULT_TIMEOUT and re-export from resolvers/utils.ts so the 5e3 timeout has a single source; use method 'POST'. - tests: cover the native-fetch error shapes in isSilencedError and the fetchHttpImage non-2xx throw. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tony8713 · 2026-06-24T09:16:27Z

Addressed the review (commit ea0a193):

Blocker — fetchHttpImage (src/resolvers/utils.ts) now checks response.ok and throws on non-2xx, so 404/500 bodies are no longer returned as image bytes; cover/logo paths fall back instead of 500-ing through sharp. Added a unit test for the non-2xx throw.

Major

fetchImageOrMetadata (src/resolvers/starknet.ts): added response.ok check + throw.
isSilencedError (src/addressResolvers/utils.ts): now inspects error.cause?.code (undici socket errors: ECONNRESET/ETIMEDOUT/ENOTFOUND/ECONNREFUSED) and error.name (AbortSignal.timeout → TimeoutError), matches codes case-insensitively. Native-fetch timeouts and socket errors are silenced again.
addressResolvers/starknet.ts: thrown error now carries response.status so transient 504s stay silenced.
Lens MUTED_ERRORS + ENS Sentry context: the GraphQL error message now includes status code <n> and error.response.data carries the response body, so MUTED_ERRORS substring matching and Sentry capture work again — no changes needed in those files.

Minor/nits

Renamed FetchError → GraphqlError in src/utils.ts to avoid the name collision with the marker class in addressResolvers/utils.ts.
response.status is now actually consumed (message + isSilencedError); also added response.data.
Hoisted DEFAULT_TIMEOUT to src/utils.ts, re-exported from resolvers/utils.ts — single source for the 5e3 timeout (utils.ts + addressResolvers/starknet.ts now use it).
method: 'post' → 'POST'.

Added unit tests for the native-fetch error shapes in isSilencedError and the fetchHttpImage non-2xx throw. Left the keepAlive/undici-pooling nit as-is (out of scope; relies on undici defaults). Also updated the PR title to "native fetch" since the code uses global fetch (Node 22), not node-fetch.

tony8713 and others added 5 commits June 10, 2026 14:21

test: drop graphQlCall unit test

f9b8222

Per review feedback, removing the fetch-mocked unit test added earlier; it was exploratory. Native fetch migration and wrapper removal stay. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

tony8713 force-pushed the refactor/axios-to-node-fetch branch from 9d1405e to f9b8222 Compare June 10, 2026 07:24

wa0x6e requested a review from Copilot June 10, 2026 10:44

Copilot started reviewing on behalf of wa0x6e June 10, 2026 10:44 View session

Copilot AI reviewed Jun 10, 2026

View reviewed changes

wa0x6e reviewed Jun 24, 2026

View reviewed changes

tony8713 changed the title ~~refactor: migrate axios to node-fetch~~ refactor: migrate axios to native fetch Jun 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

refactor: migrate axios to native fetch#457

refactor: migrate axios to native fetch#457
tony8713 wants to merge 6 commits into
masterfrom
refactor/axios-to-node-fetch

tony8713 commented Jun 9, 2026

Uh oh!

codecov Bot commented Jun 9, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

wa0x6e left a comment

Uh oh!

tony8713 commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

tony8713 commented Jun 9, 2026

What

Why now

Changes

Verification

Uh oh!

codecov Bot commented Jun 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

wa0x6e left a comment

Choose a reason for hiding this comment

Review: axios → native fetch migration

🔴 Blocker

🟡 Major

🔵 Minor / ⚪ Nit

Uh oh!

tony8713 commented Jun 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov Bot commented Jun 9, 2026 •

edited

Loading