[READY] Replace axios with native fetch wrapper#1109
Open
Sharqiewicz wants to merge 5 commits intostagingfrom
Open
[READY] Replace axios with native fetch wrapper#1109Sharqiewicz wants to merge 5 commits intostagingfrom
Sharqiewicz wants to merge 5 commits intostagingfrom
Conversation
✅ Deploy Preview for vortexfi ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for vortex-sandbox ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR migrates the codebase from axios to a native fetch-based HTTP layer, introducing typed error handling and improving request cancellation behavior (via TanStack Query AbortSignal).
Changes:
- Replaced axios usage with
fetchin shared SquidRouter + dynamic token fetch paths. - Introduced a frontend
api-clientwrapper (apiFetch/apiRequest) withApiError+isApiErrorfor typed error handling. - Wired TanStack Query
AbortSignalthrough to relevant frontend API calls (fee comparison, ramp history, fiat accounts).
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/shared/src/tokens/evm/dynamicEvmTokens.ts | Switch SquidRouter token retrieval from axios to fetch with basic HTTP status handling. |
| packages/shared/src/services/squidrouter/route.ts | Replace axios-based route/status calls with a squidFetch helper and custom HTTP error type. |
| packages/shared/package.json | Remove axios dependency from shared package. |
| package.json | Remove root axios dependency. |
| bun.lock | Remove axios entries from the lockfile. |
| apps/frontend/src/services/api/ramp.service.ts | Add optional AbortSignal to ramp history API call and forward to apiRequest. |
| apps/frontend/src/services/api/price.service.ts | Add optional AbortSignal to price API call and forward to apiRequest. |
| apps/frontend/src/services/api/monerium.service.ts | Migrate axios error checks to isApiError and adjust response handling to new client return shape. |
| apps/frontend/src/services/api/auth.api.ts | Update auth calls to use the new apiClient return shape (data directly). |
| apps/frontend/src/services/api/api-client.ts | Replace axios client with fetch wrapper, add ApiError + isApiError, and update apiRequest. |
| apps/frontend/src/services/api/alfredpay.service.ts | Update Alfredpay service methods to return data directly from apiClient; add AbortSignal to fiat accounts listing. |
| apps/frontend/src/sections/individuals/FeeComparison/FeeComparisonTable/hooks/useFeeComparisonData.ts | Pass React Query signal down to price service. |
| apps/frontend/src/hooks/useRampHistory.ts | Pass React Query signal down to ramp history service. |
| apps/frontend/src/hooks/alfredpay/useFiatAccounts.ts | Pass React Query signal down to Alfredpay fiat accounts listing. |
| apps/frontend/package.json | Remove axios dependency from frontend package. |
| apps/api/package.json | Remove axios dependency from API package. |
Comments suppressed due to low confidence (1)
apps/frontend/src/hooks/useRampHistory.ts:43
- The queryFn now receives an
AbortSignal, but aborts will be caught and swallowed by the broadcatch, causing the query to resolve successfully with partial/empty data instead of being cancelled. Consider detecting abort (signal.abortedorerror.name === "AbortError") and rethrowing so React Query can treat it as a cancellation.
queryFn: async ({ signal }) => {
const allTransactions: Transaction[] = [];
for (const address of addresses) {
try {
const response = await RampService.getRampHistory(address, 100, undefined, signal);
const transactions = response.transactions.map(formatTransaction);
allTransactions.push(...transactions);
} catch (error) {
console.warn(`Failed to fetch wallet history for ${address}:`, error);
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+4
to
+12
| export class ApiError extends Error { | ||
| status: number; | ||
| data: { error?: string; message?: string; details?: string }; | ||
|
|
||
| /** | ||
| * Base API client for making requests to the backend | ||
| */ | ||
| export const apiClient: AxiosInstance = axios.create({ | ||
| baseURL: `${SIGNING_SERVICE_URL}/v1`, | ||
| headers: { | ||
| "Content-Type": "application/json" | ||
| }, | ||
| timeout: 30000 | ||
| }); | ||
| constructor(status: number, data: Record<string, unknown>, message: string) { | ||
| super(message); | ||
| this.status = status; | ||
| this.data = data; | ||
| } |
There was a problem hiding this comment.
ApiError.data is declared as { error?: string; message?: string; details?: string }, but the constructor accepts Record<string, unknown> and assigns it directly. This is not type-safe (and likely fails TS assignability because unknown isn’t assignable to string). Consider typing data as Record<string, unknown> (or making ApiError generic) and extracting error/message/details via a helper when needed.
Comment on lines
+40
to
+49
| const response = await fetch(url.toString(), { | ||
| body: isFormData ? (options.data as FormData) : options.data !== undefined ? JSON.stringify(options.data) : undefined, | ||
| headers: { | ||
| ...(tokens?.accessToken ? { Authorization: `Bearer ${tokens.accessToken}` } : {}), | ||
| ...(!isFormData ? { "Content-Type": "application/json" } : {}), | ||
| ...options.headers | ||
| }, | ||
| method, | ||
| signal: options.signal ?? AbortSignal.timeout(30000) | ||
| }); |
There was a problem hiding this comment.
signal: options.signal ?? AbortSignal.timeout(30000) changes timeout semantics: when a caller provides a signal, the 30s timeout is no longer applied (axios previously always enforced it). Also, AbortSignal.timeout isn’t supported in all browsers/environments. Consider combining signals (e.g., timeout + caller signal via AbortSignal.any or a manual AbortController) and falling back when AbortSignal.timeout is unavailable.
Comment on lines
+4
to
66
| export class ApiError extends Error { | ||
| status: number; | ||
| data: { error?: string; message?: string; details?: string }; | ||
|
|
||
| /** | ||
| * Base API client for making requests to the backend | ||
| */ | ||
| export const apiClient: AxiosInstance = axios.create({ | ||
| baseURL: `${SIGNING_SERVICE_URL}/v1`, | ||
| headers: { | ||
| "Content-Type": "application/json" | ||
| }, | ||
| timeout: 30000 | ||
| }); | ||
| constructor(status: number, data: Record<string, unknown>, message: string) { | ||
| super(message); | ||
| this.status = status; | ||
| this.data = data; | ||
| } | ||
| } | ||
|
|
||
| export function isApiError(error: unknown): error is ApiError { | ||
| return error instanceof ApiError; | ||
| } | ||
|
|
||
| // Add request interceptor for common headers and auth token | ||
| apiClient.interceptors.request.use( | ||
| config => { | ||
| // Add Authorization header if user is authenticated | ||
| const tokens = AuthService.getTokens(); | ||
| if (tokens?.accessToken) { | ||
| config.headers.Authorization = `Bearer ${tokens.accessToken}`; | ||
| async function apiFetch<T>( | ||
| method: string, | ||
| path: string, | ||
| options: { | ||
| data?: unknown; | ||
| params?: Record<string, string | number | boolean | undefined>; | ||
| headers?: Record<string, string>; | ||
| signal?: AbortSignal; | ||
| } = {} | ||
| ): Promise<T> { | ||
| const tokens = AuthService.getTokens(); | ||
|
|
||
| const url = new URL(`${SIGNING_SERVICE_URL}/v1${path}`); | ||
| if (options.params) { | ||
| for (const [key, value] of Object.entries(options.params)) { | ||
| if (value !== undefined) url.searchParams.set(key, String(value)); | ||
| } | ||
| return config; | ||
| }, | ||
| error => { | ||
| return Promise.reject(error); | ||
| } | ||
| ); | ||
|
|
||
| // Add response interceptor for error handling | ||
| apiClient.interceptors.response.use( | ||
| response => { | ||
| return response; | ||
| }, | ||
| (error: AxiosError) => { | ||
| console.error("API Error:", error.response?.data || error.message); | ||
| return Promise.reject(error); | ||
| const isFormData = options.data instanceof FormData; | ||
|
|
||
| const response = await fetch(url.toString(), { | ||
| body: isFormData ? (options.data as FormData) : options.data !== undefined ? JSON.stringify(options.data) : undefined, | ||
| headers: { | ||
| ...(tokens?.accessToken ? { Authorization: `Bearer ${tokens.accessToken}` } : {}), | ||
| ...(!isFormData ? { "Content-Type": "application/json" } : {}), | ||
| ...options.headers | ||
| }, | ||
| method, | ||
| signal: options.signal ?? AbortSignal.timeout(30000) | ||
| }); | ||
|
|
||
| if (!response.ok) { | ||
| const errorData = (await response.json().catch(() => ({}))) as { error?: string; message?: string }; | ||
| console.error("API Error:", errorData); | ||
| throw new ApiError(response.status, errorData, errorData.error ?? errorData.message ?? response.statusText); | ||
| } | ||
| ); | ||
|
|
||
| /** | ||
| * Helper function to handle API errors | ||
| * @param error The error object | ||
| * @param defaultMessage Default error message | ||
| * @returns Formatted error message | ||
| */ | ||
| if (response.status === 204) return undefined as T; | ||
| return response.json() as Promise<T>; | ||
| } | ||
|
|
||
| export const handleApiError = (error: unknown, defaultMessage = "An error occurred"): string => { | ||
| if (axios.isAxiosError(error)) { | ||
| const responseData = error.response?.data as { error?: string; message?: string; details?: string } | undefined; | ||
| return responseData?.error || responseData?.message || error.message || defaultMessage; | ||
| if (isApiError(error)) { | ||
| return error.data?.error ?? error.data?.message ?? error.message ?? defaultMessage; | ||
| } | ||
| return error instanceof Error ? error.message : defaultMessage; | ||
| }; |
There was a problem hiding this comment.
The new client now throws ApiError (with status/data) rather than an axios error shape (error.response.status/data). There are still call sites in the frontend that inspect err.response to branch on status/body, which will break with this change. Either update those call sites or consider exposing a backward-compatible shape/helper to reduce migration risk.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary