chore: Replace axios with native Node.js fetch#3134
chore: Replace axios with native Node.js fetch#3134nicholaspai wants to merge 8 commits intomasterfrom
Conversation
nicholaspai
requested review from
bmzig,
dohaki,
mrice32 and
pxrl
as code owners
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7a6ecaa152
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Avoid parsing body and caching all-zero reserves when the liquid-reserves API returns a non-2xx status (e.g. 429/5xx transient failures). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-authored-by: Paul <108695806+pxrl@users.noreply.github.com>
| } | ||
|
|
||
| // No axios error, process `proofState` | ||
| proofState = (await getResponse.json()) as ProofStateResponse; |
There was a problem hiding this comment.
We probably want to follow up and check how to validate types on this. I'm not really sure why it wasn't required for; perhaps the return was typed as any 🤔
There was a problem hiding this comment.
The ProofStateResponse type from src/interfaces/ZkApi.ts is already well-defined with typed status, update_calldata, and error_message fields. The as ProofStateResponse cast on the response.json() result gives us the same type safety as the prior axios.get<ProofStateResponse> generic — both are essentially trust-the-server assertions since neither validates at runtime. If we want true runtime validation we'd need a schema library (zod, etc), but that's a bigger lift.
pxrl
left a comment
pxrl
left a comment
There was a problem hiding this comment.
@nicholaspai I wonder if we should be wrapping fetch to accept generic typing hints supplied by the caller. It might make our use of fetch a bit more consistent. Not sure if it's the right approach though.
- Use isDefined() instead of manual null/undefined check in AcrossApiBaseClient - Remove unused stringifyThrownValue import in helios.ts - Add proper types for Native Markets API responses in Refiller (remove `any`) - Add proper type for Tenderly simulation response in simulateFill - Skip caching all-zero reserves in AcrossAPIClient to avoid persisting bad data from transient API failures Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b9e39a45e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Resolve yarn.lock conflict by regenerating lockfile. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
| try { | ||
| const response = await axios.get(url, { headers, responseType: "text", timeout: 30000 }); | ||
| return response.data as string; | ||
| const response = await fetch(url, { | ||
| headers, | ||
| signal: AbortSignal.timeout(30000), | ||
| }); | ||
| if (!response.ok) { | ||
| throw new Error(`HTTP ${response.status}: ${response.statusText}`); | ||
| } | ||
| return await response.text(); | ||
| } catch (error) { |
There was a problem hiding this comment.
wdyt about getting rid of this try/catch construction? fetch won't throw by default so it's basically artificial that we're setting up the try/catch loop and then are the sole reason for it to throw. i.e. it should be possible to just evaluate response.status directly, rather than doing string comparisons on error.message.
2 participants
Summary
axiosimports and usage across 10 files in the codebasefetchAPI throughout:axios.get()→fetch()with response status checksaxios.post()→fetch()withmethod: "POST"andJSON.stringify()axios.isAxiosError()→ HTTP status code checks on the responsetimeoutconfig →AbortSignal.timeout()paramsconfig →URLSearchParamsappended to URLRawAxiosRequestHeaders/AxiosErrortypes →Record<string, string>/Erroraxiosfrompackage.jsondependencies and updatedyarn.lockFiles changed
src/utils/BridgeUtils.ts— GET/POST with retry logicsrc/utils/CCTPUtils.ts— Circle CCTP attestation fetchessrc/utils/OFTUtils.ts— LayerZero transaction detailssrc/clients/AcrossApiBaseClient.ts— Base API client with timeout/params/authsrc/clients/AcrossAPIClient.ts— Liquid reserves fetchingsrc/finalizer/utils/scroll.ts— Scroll unclaimed withdrawals APIsrc/finalizer/utils/helios.ts— ZK proof API (GET/POST with 404 handling)src/refiller/Refiller.ts— Native markets API (addresses, transfer routes)scripts/simulateFill.ts— Tenderly simulation APIscripts/fetchInventoryConfig.ts— Configurama config fetchingTest plan
tsc --noEmitpasses (confirmed locally)🤖 Generated with Claude Code