chore(auth): replace axios in auth/customs with native fetch#20735
Open
toufali wants to merge 1 commit into
Open
chore(auth): replace axios in auth/customs with native fetch#20735toufali wants to merge 1 commit into
toufali wants to merge 1 commit into
Conversation
a959c64 to
a221325
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR continues the migration away from axios/agentkeepalive in fxa-auth-server by switching the legacy customs client and the Google RISC events CLI to native fetch, while simplifying the related configuration surface and modernizing the customs unit tests.
Changes:
- Replaced
axios+agentkeepaliveusage inlib/customs.jswith nativefetch+AbortSignal.timeout, preserving fail-closed behavior. - Simplified customs configuration from socket-pool/agent settings to a single
customsClient.timeoutMsvalue. - Reworked
lib/customs.spec.tsto stubglobal.fetchinstead of usingnock, and removed the now-unusedhttp-agent.ts+ dependency.
Reviewed changes
Copilot reviewed 6 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| yarn.lock | Removes agentkeepalive from the auth-server workspace dependency graph. |
| packages/fxa-auth-server/scripts/google-events-cli.js | Switches Google RISC CLI requests from axios to fetch. |
| packages/fxa-auth-server/package.json | Drops the agentkeepalive dependency from auth-server. |
| packages/fxa-auth-server/lib/http-agent.ts | Deletes the custom agent factory now that fetch is used. |
| packages/fxa-auth-server/lib/customs.spec.ts | Updates tests to stub global.fetch and adds focused unit coverage for payload sanitization. |
| packages/fxa-auth-server/lib/customs.js | Migrates customs HTTP calls to fetch, adds request timeout via AbortSignal.timeout. |
| packages/fxa-auth-server/config/index.ts | Replaces customsHttpAgent config with customsClient.timeoutMs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
a221325 to
f992749
Compare
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.
Because
axiosis a recurring source of security advisories. Continues the migration to nativefetch(FXA-13427); this PR covers the customs client and the Google RISC events CLI, and removes the now-unusedagentkeepalivefrom auth-server.This pull request
customs.js) fromaxios+agentkeepaliveto nativefetch, preserving the request timeout viaAbortSignal.timeoutand the fail-closed error handling (backendServiceFailure).customsHttpAgentconfig (socket-pool knobs) to a singlecustomsClient.timeoutMs(retaining theCUSTOMS_TIMEOUT_MSenv binding); deletes the orphanedhttp-agent.tsand theagentkeepalivedependency.customs.spec.tsoffnockontoglobal.fetchstubs, simplifying it (753 → 569 lines) into focused one-behavior tests plus a directsanitizePayloadunit test.google-events-cli.jsRISC script offaxios.Issue that this pull request solves
Closes: FXA-13427 (customs + scripts slice)
Checklist
Put an
xin the boxes that applyOther information (Optional)
Connection pooling:
fetch(undici) pools and keep-alives connections by default. The droppedmaxSocketscaps were only ever raised under load testing (the historical pain was too few sockets), and undici's default per-origin pool is unlimited — so moving to defaults goes in the safe direction. The 8s prod timeout (CUSTOMS_TIMEOUT_MS) is preserved.Ops follow-ups (not blocking this PR):
customs.{httpAgent,httpsAgent}.*StatsD socket gauges are removed —fetch/undici exposes no agent to introspect. Heads-up for anyone with customs Grafana panels/alerts on those metrics.CUSTOMS_MAX_SOCKETS,CUSTOMS_MAX_FREE_SOCKETS, andCUSTOMS_FREE_SOCKET_TIMEOUT_MSenv vars are now unused.webservices-infrastill setscustoms_free_socket_timeout_ms(now inert) — worth removing there in a follow-up.axiositself remains in the rootpackage.jsonuntillibs/profile/clientmigrates (next PR).